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 of g602 with maps #1005

Closed
david-gang opened this issue Aug 22, 2023 · 10 comments · Fixed by #1017
Closed

false positive of g602 with maps #1005

david-gang opened this issue Aug 22, 2023 · 10 comments · Fixed by #1017
Labels

Comments

@david-gang
Copy link

Summary

When setting keys in maps which obviously fon't exist before we get an error message

Steps to reproduce the behavior

filter := make(map[string]any, 0)
		filter["term"] = map[string]interface{}{
			filedName: map[string]interface{}{
				"value": 5,
			},
		}
		res = append(res, filter)
	}

gosec version

v2.17.0

Go version (output of 'go version')

GitHub Actions / golangci
golangci-lint-1.54.2, go 1.19

Operating system / Environment

ubuntu/ gh actions

Expected behavior

no error

Actual behavior

gives error

@ccojocar
Copy link
Member

This looks like a false positive since the initial capacity of the map doesn't restrict its size.

@morgenm Please could you have a look at this issue, and exclude this use case from the G602 check? Thanks

@ccojocar ccojocar added the bug label Aug 22, 2023
@sverdlov93
Copy link

+1

@micronull
Copy link

It's the same with the slice.
This code will give a false positive:

	if len(items) > 0 {
		return items[0], nil
	}

@ccojocar
Copy link
Member

@micronull could you please specify how you create the items slice? Thanks

@Goryudyuma
Copy link

@ccojocar

I don't know what it has to do with how it's made.

At least the example shown below is normal and common code, and I think it's safe to say it's a false positive.

(I used google translate)

package main

import (
	"errors"
)

func main() {
	items := make([]string, 0)
	items = append(items, "a")
	a, err := f(items)
	if err != nil {
		panic(err)
	}
	println(a)
}

func f(items []string) (string, error) {
	if len(items) > 0 {
		return items[0], nil
	}
	return "", errors.New("error")
}

Results:


[***/test/main.go:19] - G602 (CWE-118): Potentially accessing slice out of bounds (Confidence: MEDIUM, Severity: MEDIUM)
    18: 	if len(items) > 0 {
  > 19: 		return items[0], nil
    20: 	}



Summary:
  Gosec  : 2.17.0
  Files  : 1
  Lines  : 22
  Nosec  : 0
  Issues : 1

@micronull
Copy link

@micronull could you please specify how you create the items slice? Thanks

	items := make([]T, 0)

	if err := jsoniter.NewDecoder(res.Body).Decode(&items); err != nil {
		return nil, fmt.Errorf(`%w: %w`, ErrDecodeResponse, err)
	}
       
       if len(items) == 0 {
		return nil, ErrEmptyResponse
	}

	return &items[0], nil

@phisco
Copy link

phisco commented Aug 24, 2023

same here:

func firstNAndSomeMore(names []string) string {
	if len(names) > 3 {
		return fmt.Sprintf("%s, and %d more", strings.Join(names[:3], ", "), len(names)-3)
	}
	return strings.Join(names, ", ")
}

is reported as an error through golangci-lint:

$ golangci-lint --version
golangci-lint has version 1.54.2 built with go1.21.0 from 411e0bb on 2023-08-21T11:04:00Z

@MrJoy
Copy link

MrJoy commented Aug 25, 2023

@ccojocar I'm having the same issue with slices.

In my case, I have code that looks like this:

candidates := make([]string, 0)
for _, template := range templates {
	if strings.HasPrefix(template.Name, namePrefix) {
		candidates = append(candidates, template.Name)
	}
}

if len(candidates) == 0 {
	util.Fatalf(NoMatchingLaunchTemplates, "No matching launch templates found.\n")
} else if len(candidates) > 1 {
	util.Fatalf(MultipleLaunchTemplates, "Multiple launch templates found.\n")
}

name := candidates[0]

Even if I wrap the last line up like so, I still get the error:

var name string
if len(candidates) > 0 {
	name = candidates[0]
}

@niceoneallround
Copy link

I have a similar issue using an index into a slice that should be ok.

//Check if all strings are the same
func allSameStrings(a []string) bool {
	for _, v := range a {
		if v != a[0] {
			return false
		}
	}
	return true
}

Command used is
gosec -quiet -fmt=json -confidence=medium -tests=true ./...

Message

		{
			"severity": "MEDIUM",
			"confidence": "MEDIUM",
			"cwe": {
				"id": "118",
				"url": "https://cwe.mitre.org/data/definitions/118.html"
			},
			"rule_id": "G602",
			"details": "Potentially accessing slice out of bounds",
			"file": "removed from public",
			"code": "865: \tfor _, v := range a {\n866: \t\tif v != a[0] {\n867: \t\t\treturn false\n",
			"line": "866",
			"column": "11",
			"nosec": false,
			"suppressions": null
		},

@painhardcore
Copy link

painhardcore commented Aug 29, 2023

I know that I probably should make a different Issue, but also having issues with slices on latest golangci-lint 1.54.2

	validationErrors := make([]error, 0)
        // some code
	if len(validationErrors) == 1 {
		return validationErrors[0] // this one false positive
	}

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