Skip to content
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

False Positive for G602 with bounds check using switch #1250

Open
theory opened this issue Nov 10, 2024 · 1 comment
Open

False Positive for G602 with bounds check using switch #1250

theory opened this issue Nov 10, 2024 · 1 comment
Labels

Comments

@theory
Copy link

theory commented Nov 10, 2024

Summary

I'm getting false positives for G602 when using a switch statement for bounds checking.

Steps to reproduce the behavior

Run gosec against this test case:

func main() {
	args := []any{"1"}
	switch len(args) - 1 {
	case 1:
		_ = args[1]
	}
}

Output:

[/Users/david/Downloads/try-gosec/main.go:7] - G602 (CWE-118): slice index out of range (Confidence: HIGH, Severity: LOW)
    6: 	case 1:
  > 7: 		_ = args[1]
    8: 	}

Autofix: 

Summary:
  Gosec  : dev
  Files  : 1
  Lines  : 9
  Nosec  : 0
  Issues : 1

gosec version

Just installed 1fb6a46 from GitHub.

Go version (output of 'go version')

go version go1.23.2 darwin/arm64

Operating system / Environment

macOS Sequoia

Expected behavior

No issues found.

Actual behavior

False positive for G602.

@ccojocar ccojocar added the bug label Nov 11, 2024
theory added a commit to theory/jsonpath that referenced this issue Nov 13, 2024
*   Add `spec.Filter.Eval` to allow public evaluation of a single JSON
    node. Used internally by `spec.FilterSelector.Select`.
*   Add `spec.Segment.IsDescendant` to tell wether a segments selects
    just from the current child node or also recursively selects from
    all of its descendants.
*   Make `spec.SliceSelector.Bounds` public.
*   Make the underlying struct defining `spec.Wildcard` public with the
    name `spec.WildcardSelector`.

Other changes:

*   Add missing "?" to the stringification of `spec.FilterSelector`.
*   Upgrade to `golangci-lint` v1.62 and disable `gosec` G602 false
    positives (securego/gosec#1250)
@theory
Copy link
Author

theory commented Nov 19, 2024

FWIW, I created this the case based on this code:

func Slice(args ...any) SliceSelector {
	const (
		startArg = 0
		endArg   = 1
		stepArg  = 2
	)
	// Set defaults.
	s := SliceSelector{0, math.MaxInt, 1}
	switch len(args) - 1 {
	case stepArg:
		//nolint:gosec // disable G602 https://github.com/securego/gosec/issues/1250
		switch step := args[stepArg].(type) {
		case int:
			s.step = step
		case nil:
			// Nothing to do
		default:
			panic("Third value passed to NewSlice is not an integer")
		}
		fallthrough
	case endArg:
		//nolint:gosec // disable G602 https://github.com/securego/gosec/issues/1250
		switch end := args[endArg].(type) {
		case int:
			s.end = end
		case nil:
			// Negative step: end with minimum int.
			if s.step < 0 {
				s.end = math.MinInt
			}
		default:
			panic("Second value passed to NewSlice is not an integer")
		}
		fallthrough
	case startArg:
		switch start := args[startArg].(type) {
		case int:
			s.start = start
		case nil:
			// Negative step: start with maximum int.
			if s.step < 0 {
				s.start = math.MaxInt
			}
		default:
			panic("First value passed to NewSlice is not an integer")
		}
	}
	return s
}

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

No branches or pull requests

2 participants