Skip to content

remove G113. It only affects old/unsupported versions of Go #1328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 3, 2025

Conversation

niij
Copy link
Contributor

@niij niij commented Apr 1, 2025

Newer versions of go (>=1.16.14, >=1.17.7, 1.18+) are not affected by this. Don't warn at all on those newer versions. See https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-23772

Newer versions of go (>=1.16.14, >=1.17.7, 1.18+) are not affected by this. Don't warn at all on those newer versions. See https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-23772
@niij niij requested a review from ccoVeille April 1, 2025 23:59
@ccojocar
Copy link
Member

ccojocar commented Apr 2, 2025

Thanks for this contribution but we don't support these versions anymore.

@ccojocar
Copy link
Member

ccojocar commented Apr 2, 2025

I think we can deprecate complete the check.

@ccojocar
Copy link
Member

ccojocar commented Apr 2, 2025

@niij There is a test failing. Please can you update it, and I would suggest to remove completely the check. Thanks

@niij
Copy link
Contributor Author

niij commented Apr 2, 2025

Oh I see why it's failing. Since it expects to catch this failure at some severity but the tests are running on the latest version of Go, so it's excluded.

Go 1.17 went out of support August 2022. Seems reasonable to drop support for this version-specific issue at this point. If you're compiling old versions of 1.16/1.17 and processing untrusted user input through them at this point you likely have bigger problems :)

I'll delete it today.

@niij niij changed the title don't warn on G113 (big.Rat SetString) if on an unaffected version of Go remove G113. It only affects old/unsupported versions of Go Apr 2, 2025
@niij niij requested review from ccojocar and ccoVeille April 2, 2025 17:26
@niij niij requested a review from ccojocar April 3, 2025 13:01
@ccojocar
Copy link
Member

ccojocar commented Apr 3, 2025

@niij There is a lint issue. Please could you fix it? Thanks

@niij niij marked this pull request as draft April 3, 2025 13:29
@niij niij marked this pull request as ready for review April 3, 2025 14:33
@niij
Copy link
Contributor Author

niij commented Apr 3, 2025

@ccojocar gofmt done

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.18%. Comparing base (1216c9b) to head (beefbee).
Report is 57 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1328      +/-   ##
==========================================
- Coverage   68.49%   63.18%   -5.32%     
==========================================
  Files          75       74       -1     
  Lines        4384     5175     +791     
==========================================
+ Hits         3003     3270     +267     
- Misses       1233     1778     +545     
+ Partials      148      127      -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ccojocar ccojocar merged commit 1336dc6 into securego:master Apr 3, 2025
6 checks passed
@niij niij deleted the patch-1 branch April 3, 2025 14:44
@niij
Copy link
Contributor Author

niij commented Apr 3, 2025

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants