Skip to content

Add a rule to detect implicit aliasing issues with RangeStmt's #438

@caccavale

Description

@caccavale

Summary

Elements yielded from RangeStmt's have the same memory address. Not realizing this and storing these addresses for later use is an unsafe behavior.

Steps to reproduce the behavior

package main

import "fmt"

var vector []*string

func appendVector(s *string) {
	vector = append(vector, s)
}

func printVector() {
	for _, item := range vector {
		fmt.Printf("%s", *item)
	}
	fmt.Println()
}

func foo() (int, *string, string) {
	for _, item := range vector {
		return 0, &item, item
	}
}

func main() {
	for _, item := range []string{"A", "B", "C"} {
		appendVector(&item)
	}

	printVector()

	zero, c_star, c := foo()
	fmt.Printf("%d %v %s", zero, c_start, c)
}

Expected behavior

foo's behavior shouldn't cause any issues, but appendVector(&item) will.
I propose adding the following rule:

func (r *implicitAliasing) Match(n ast.Node, c *gosec.Context) (*gosec.Issue, error) {
	if rangeStmt, ok := n.(*ast.RangeStmt); ok && rangeStmt.Value != nil && rangeStmt.Tok.String() == ":=" {
		if ident, ok := rangeStmt.Value.(*ast.Ident); ok {
			name := ident.Name

			var issue *gosec.Issue
			var fun func(n ast.Node) bool
			fun = func(n ast.Node) bool {
				if ret, ok := n.(*ast.ReturnStmt); ok {
					for _, element := range ret.Results {
						if isDereferenceOf(element, name) {
							continue
						}

						ast.Inspect(element, fun)
						if issue != nil {
							return false
						}
					}

					return false
				}

				if isDereferenceOf(n, name) {
					issue = gosec.NewIssue(c, n, r.ID(), r.What, r.Severity, r.Confidence)
					return false
				}

				return issue == nil
			}

			ast.Inspect(rangeStmt.Body, fun)
			return issue, nil
		}
	}

	return nil, nil
}

It does have false positives but seems like it is a good stab of what can be a nasty bug. I haven't participated in much open source software so please advise on how I should go about this; I can open a PR with the work I have done if that is desired. Thanks.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions