Merge branch 'main' into lucas

.github/workflows/semgrep-rules-test.yml

@@ -19,6 +19,6 @@ jobs:
           python -m pip install --upgrade pip
           python3 -m pip install semgrep
       - name: validations
-        run: python -m semgrep --validate --config .
+        run: semgrep --validate --config .
       - name: tests
-        run: python -m semgrep --test --test-ignore-todo
+        run: semgrep --test --test-ignore-todo

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{
 		{

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 ...>
+        }
   

go/racy-append-to-slice.go

@@ -23,6 +23,9 @@ func main() {
 
 	res6 := appendToSlice_FP4()
 	printResults(res6)
+
+	res7 := appendToSlice_FP5()
+	printResults(res7)
 }
 
 func appendToSlice1() []int {
@@ -71,8 +74,8 @@ func appendToSlice_FP1() []int {
 		go func(iCpy int) {
 			defer wg.Done()
 			m := iCpy * 2
-			// ok: racy-append-to-slice
 			rMut.Lock()
+			// ok: racy-append-to-slice
 			r = append(r, m)
 			rMut.Unlock()
 		}(i)
@@ -146,6 +149,29 @@ func appendToSlice_FP4() []int {
 	return r
 }
 
+func appendToSlice_FP5() []int {
+	// FP: The `append` is done inside a defer lock
+	var wg sync.WaitGroup
+	var rMut sync.Mutex
+	var r []int
+	for i := 0; i < 10; i++ {
+		wg.Add(1)
+		go func(iCpy int) {
+			defer wg.Done()
+			m := iCpy * 2
+			rMut.Lock()
+			fmt.Println("test")
+			defer rMut.Unlock()
+			// ok: racy-append-to-slice
+			r = append(r, m)
+		}(i)
+	}
+
+	wg.Wait()
+
+	return r
+}
+
 func printResults(results []int) {
 	for v, _ := range results {
 		fmt.Println(v)

go/racy-append-to-slice.yaml

@@ -48,3 +48,8 @@ rules:
           $MUTEX.Lock()
           ...
           $MUTEX.Unlock()
+    - pattern-not-inside: |
+          $MUTEX.Lock()
+          ...
+          defer $MUTEX.Unlock()
+          ...