KEMBAR78
remove G113. It only affects old/unsupported versions of Go by niij · Pull Request #1328 · securego/gosec · GitHub
Skip to content

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
Co-authored-by: ccoVeille <3875889+ccoVeille@users.noreply.github.com>
@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 ccoVeille and ccojocar 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