check nil slices, partially check bounds #1396
Conversation
There was a problem hiding this comment.
Thank you for contributing this fix. Please add some tests in the rule test file to cover this use case https://github.com/securego/gosec/blob/master/testutils/g602_samples.go.
analyzers/slice_bounds.go
Outdated
| case *ssa.Store: // check store to nil slice or out of bounds | ||
| case *ssa.IndexAddr: | ||
| if constantValue, ok := instr.X.(*ssa.Const); ok && constantValue.Type().String()[:2] == "[]" { | ||
| if constantValue.Value == nil { |
There was a problem hiding this comment.
Shouldn't this check be placed before accessing the .Type()? What happens if is already nil?
There was a problem hiding this comment.
The code assumes that someone will continue to search for errors. For example, when var a [5]int this const value not nil, in this case the simplest error is determined when instead of append(a,...) index assignment is used.
analyzers/slice_bounds.go
Outdated
| } | ||
| case *ssa.Store: // check store to nil slice or out of bounds | ||
| case *ssa.IndexAddr: | ||
| if constantValue, ok := instr.X.(*ssa.Const); ok && constantValue.Type().String()[:2] == "[]" { |
There was a problem hiding this comment.
Can the type length be less than 2 under any circumstances?
There was a problem hiding this comment.
if type name string less then []*, then this is not slice in this case
|
@ccojocar review please |
|
There are some lint warnings which need to be fixed before merging, otherwise LGTM. |
|
It seems that there is a test failure. |
|
There are still some issues when the gosec built from this pull request. PTAL. Thanks |
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1396 +/- ##
==========================================
- Coverage 68.49% 63.01% -5.49%
==========================================
Files 75 76 +1
Lines 4384 5443 +1059
==========================================
+ Hits 3003 3430 +427
- Misses 1233 1878 +645
+ Partials 148 135 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
fix #1392 partially