diff --git a/go/invalid-usage-of-modified-variable.go b/go/invalid-usage-of-modified-variable.go index 842ebaf..6615d47 100644 --- a/go/invalid-usage-of-modified-variable.go +++ b/go/invalid-usage-of-modified-variable.go @@ -12,6 +12,10 @@ type Engineer struct { Address *Address } +func (e *Engineer) String() string { + return e.FName + " " + e.LName +} + type Address struct { City string State string @@ -24,40 +28,240 @@ type Log struct { func main() { engineers := getEngineers() + + // BASIC TESTS // ruleid: invalid-usage-of-modified-variable eng1, err := getEngineerAtIndex(engineers, 5) if err != nil { fmt.Printf("Unable to obtain engineer with FName %s\n", eng1.FName) } + // ruleid: invalid-usage-of-modified-variable + eng11, err2 := getEngineerAtIndex(engineers, 5) + if err2 != nil { + fmt.Printf("Something") + fmt.Printf("Unable to obtain engineer with FName %s\n", eng11.FName) + fmt.Printf("Something") + } + + var eng12 *Engineer + // ruleid: invalid-usage-of-modified-variable + eng12, err = getEngineerAtIndex(engineers, 5) + if err != nil { + fmt.Printf("Unable to obtain engineer with FName %s\n", eng12.FName) + } + + var eng13 *Engineer + // ruleid: invalid-usage-of-modified-variable + eng13, err = getEngineerAtIndex(engineers, 5) + if err != nil { + fmt.Printf("Something") + fmt.Printf("Unable to obtain engineer with FName %s\n", eng13.FName) + fmt.Printf("Something") + } + + // ruleid: invalid-usage-of-modified-variable + eng14, addr1, err := getEngineerAndAddressByIndex(engineers, 5) + if err != nil { + fmt.Printf("Something") + fmt.Printf("Unable to obtain engineer with FName %s\n", eng14.FName) + fmt.Printf("Something") + } + + // BASIC TESTS - inline if // ruleid: invalid-usage-of-modified-variable if eng2, err := getEngineerAtIndex(engineers, 6); err != nil { fmt.Printf("Unable to obtain engineer with FName %s\n", eng2.FName) } - eng3 := Engineer{4, "Lee", "Renaldo", 50, nil} // ruleid: invalid-usage-of-modified-variable + if eng21, err := getEngineerAtIndex(engineers, 6); err != nil { + fmt.Printf("Something") + fmt.Printf("Unable to obtain engineer with FName %s\n", eng21.FName) + fmt.Printf("Something") + } + + var eng22 *Engineer + // ruleid: invalid-usage-of-modified-variable + if eng22, err = getEngineerAtIndex(engineers, 6); err != nil { + fmt.Printf("Unable to obtain engineer with FName %s\n", eng22.FName) + } + + var eng23 *Engineer + // ruleid: invalid-usage-of-modified-variable + if eng23, err = getEngineerAtIndex(engineers, 6); err != nil { + fmt.Printf("Something") + fmt.Printf("Unable to obtain engineer with FName %s\n", eng23.FName) + fmt.Printf("Something") + } + + var eng24 *Engineer + // ruleid: invalid-usage-of-modified-variable + if eng24, err = getEngineerAtIndex(engineers, 6); err != nil { + fmt.Printf("Something") + fmt.Printf("Unable to obtain engineer with FName %s\n", eng24.FName) + fmt.Printf("Something") + eng24 = &Engineer{0, "N/A", "N/A", 0, nil} + } + + // WHOLE VAR AND METHODS TESTS + // ruleid: invalid-usage-of-modified-variable + eng5, err := getEngineerAtIndex(engineers, 5) + if err != nil { + fmt.Printf("Something") + fmt.Printf("%s\n", eng5.String()) + fmt.Printf("Something") + } + + + // FALSE POSITIVES + eng3 := Engineer{4, "Lee", "Renaldo", 50, nil} + // ok: invalid-usage-of-modified-variable eng3.Address, err = getEngineerAddressByIndex(engineers, 1) if err != nil { fmt.Printf("Unable to obtain address %#v!\n", eng3.Address) } - // ruleid: invalid-usage-of-modified-variable + // ok: invalid-usage-of-modified-variable + eng3.Address, err = getEngineerAddressByIndex(engineers, 1) + if err != nil { + fmt.Printf("Something") + fmt.Printf("Unable to obtain address %#v!\n", eng3.Address) + fmt.Printf("Something") + } + + // ok: invalid-usage-of-modified-variable eng4, err := getEngineerAtIndex(engineers, 5) if err != nil { - log := Log{ - Id: eng4.Id, - Message: "Unable to obtain engineer", - } - fmt.Printf("%#v\n", log) + fmt.Printf("Something") + fmt.Printf("%#v\n", eng4) + fmt.Printf("Something") + } + + // ok: invalid-usage-of-modified-variable + eng6, err := getEngineerAtIndex(engineers, 7) + if err != nil { + eng6 = &Engineer{0, "N/A", "N/A", 0, nil} + fmt.Printf("Unable to obtain engineer %d!\n", eng6.Id) + } + + // ok: invalid-usage-of-modified-variable + eng61, err := getEngineerAtIndex(engineers, 7) + if err != nil { + fmt.Printf("Something") + eng61 = &Engineer{0, "N/A", "N/A", 0, nil} + fmt.Printf("Unable to obtain engineer %d!\n", eng61.Id) + fmt.Printf("Something") + } + + // ok: invalid-usage-of-modified-variable + eng7, err := getEngineerAtIndex(engineers, 8) + if err != nil { + eng7 := &Engineer{0, "N/A", "N/A", 0, nil} + fmt.Printf("Unable to obtain engineer %d!\n", eng7.Id) + } + + // ok: invalid-usage-of-modified-variable + eng71, err := getEngineerAtIndex(engineers, 8) + if err != nil { + fmt.Printf("Something") + eng71 := &Engineer{0, "N/A", "N/A", 0, nil} + fmt.Printf("Unable to obtain engineer %d!\n", eng71.Id) + fmt.Printf("Something") + } + + + var eng8 *Engineer + // ok: invalid-usage-of-modified-variable + eng8, err = getEngineerAtIndex(engineers, 7) + if err != nil { + eng8 = &Engineer{0, "N/A", "N/A", 0, nil} + fmt.Printf("Unable to obtain engineer %d!\n", eng8.Id) } + var eng81 *Engineer // ok: invalid-usage-of-modified-variable - if eng5, err := getEngineerAtIndex(engineers, 6); err == nil { - fmt.Printf("Obtained engineer with FName %s\n", eng5.FName) + eng81, err = getEngineerAtIndex(engineers, 7) + if err != nil { + fmt.Printf("Something") + eng81 = &Engineer{0, "N/A", "N/A", 0, nil} + fmt.Printf("Unable to obtain engineer %d!\n", eng81.Id) + fmt.Printf("Something") + } + + // ok: invalid-usage-of-modified-variable + eng9, err := getEngineerAtIndex(engineers, 8) + if err != nil { + eng9 = &Engineer{0, "N/A", "N/A", 0, nil} + } + + var eng91 *Engineer + // ok: invalid-usage-of-modified-variable + eng91, err = getEngineerAtIndex(engineers, 8) + if err != nil { + eng91 = &Engineer{0, "N/A", "N/A", 0, nil} + } + + // ok: invalid-usage-of-modified-variable + eng92, addr2, err := getEngineerAndAddressByIndex(engineers, 5) + if err != nil { + fmt.Printf("Something") + eng92, err = getEngineerAtIndex(engineers, 6) + fmt.Printf("Something") + } + + // ok: invalid-usage-of-modified-variable + eng93, err := getEngineerAtIndex(engineers, 8) + if err != nil { + eng93 = &Engineer{0, "N/A", "N/A", 0, nil} + eng93 = &Engineer{eng93.Id, "N/A", "N/A", 0, nil} + } + + + // TRUE POSITIVES + + // ruleid: invalid-usage-of-modified-variable + eng11, err = getEngineerAtIndex(engineers, 8) + if err != nil { + eng11.Address = nil + } + + // ruleid: invalid-usage-of-modified-variable + eng12, err = getEngineerAtIndex(engineers, 8) + if err != nil { + eng12 = &Engineer{eng12.Id, "N/A", "N/A", 0, nil} + } + + // The following test case should match, but I was unable to find a way to + // match it without causing some of the false positives to match. This is + // largely due to the requirement that "<... $X.$Y ...>" be the last line in + // the if-block, i.e. the following causes an invalid pattern error: + // - pattern: | + // $X, err = ... + // if err != nil { + // ... + // <... $X.$Y ...> + // ... + // $X = ... + // } + + // todoruleid: invalid-usage-of-modified-variable + eng10, err := getEngineerAtIndex(engineers, 10) + if err != nil { + fmt.Printf("Something") + fmt.Printf("Unable to obtain engineer %d!\n", eng10.Id) + eng10 = &Engineer{0, "N/A", "N/A", 0, nil} + fmt.Printf("Unable to obtain engineer %d!\n", eng10.Id) } fmt.Printf("Engineer 1: %s", fmt.Sprintf("%s %s", eng1.FName, eng1.LName)) + fmt.Printf("Engineer 7: %s", fmt.Sprintf("%s %s", eng7.FName, eng7.LName)) + fmt.Printf("Engineer 71: %s", fmt.Sprintf("%s %s", eng7.FName, eng71.LName)) + fmt.Printf("Engineer 9: %s", fmt.Sprintf("%s %s", eng9.FName, eng9.LName)) + fmt.Printf("Engineer 91: %s", fmt.Sprintf("%s %s", eng91.FName, eng91.LName)) + fmt.Printf("Engineer 92: %s", fmt.Sprintf("%s %s", eng92.FName, eng92.LName)) + fmt.Printf("Address 1: %v", addr1) + fmt.Printf("Address 2: %v", addr2) } func getEngineerAtIndex(slice []Engineer, idx int) (*Engineer, error) { @@ -74,6 +278,13 @@ func getEngineerAddressByIndex(slice []Engineer, idx int) (*Address, error) { return slice[idx].Address, nil } +func getEngineerAndAddressByIndex(slice []Engineer, idx int) (*Engineer, *Address, error) { + if idx >= len(slice) { + return nil, nil, fmt.Errorf("invalid index") + } + return &slice[idx], slice[idx].Address, nil +} + func getEngineers() []Engineer { return []Engineer{ { diff --git a/go/invalid-usage-of-modified-variable.yaml b/go/invalid-usage-of-modified-variable.yaml index 95922a4..7f631e8 100644 --- a/go/invalid-usage-of-modified-variable.yaml +++ b/go/invalid-usage-of-modified-variable.yaml @@ -18,16 +18,27 @@ rules: - https://blog.trailofbits.com/2019/11/07/attacking-go-vr-ttps/ patterns: - - pattern-either: - - pattern: | - $X, err = ... - if err != nil { - <... $X ...> - } - - pattern: | - $X, err := ... - if err != nil { - ... - <... $X.$Y ...> - } + - pattern: | + ..., $X, ..., $ERR = ... + if $ERR != nil { + ... + <... $X.$Y ...> + } + + - pattern-not: | + ..., $X, ..., $ERR = ... + if $ERR != nil { + ... + $X, ... = ... + ... + <... $X.$Y ...> + } + - pattern-not: | + ..., $X, ..., $ERR = ... + if $ERR != nil { + ... + $X = ... + ... + <... $X.$Y ...> + } \ No newline at end of file