Merge pull request #59 from trailofbits/go-lock-fixes

Reduce FPs in `missing-runlock-on-rwmutex` and `missing-unlock-before-return`

go/missing-runlock-on-rwmutex.go

@@ -14,6 +14,14 @@ type RWContainer struct {
 	counters map[string]int
 }
 
+type Fridge struct {
+	food int
+}
+
+func (f *Fridge) RLock() {
+	fmt.Println(f.food)
+}
+
 func main() {
 	c := RWContainer{
 		counters: map[string]int{"a": 0, "b": 0},
@@ -53,6 +61,7 @@ func (c *RWContainer) inc(name string) error {
 		return fmt.Errorf("letter not allowed")
 	}
 	c.mu.RUnlock()
+	// ok: missing-runlock-on-rwmutex
 	return nil
 }
 
@@ -76,6 +85,7 @@ func (c *RWContainer) inc_FP(name string) error {
 		return fmt.Errorf("letter not allowed")
 	}
 	c.mu.RUnlock()
+	// ok: missing-runlock-on-rwmutex
 	return nil
 }
 
@@ -87,3 +97,78 @@ func (c *RWContainer) inc4FP(name string) RUnlocker {
 		c.mu.Unlock()
 	}
 }
+
+func (c *RWContainer) inc5(name string) error {
+	f := Fridge{food: 11}
+	f.RLock()
+	// ok: missing-runlock-on-rwmutex
+	return nil
+}
+
+func (c *RWContainer) inc6(name string) error {
+	c.mu.RLock()
+	c.counters[name]++
+	defer func() {
+		fmt.Println("before runlock")
+		c.mu.RUnlock()
+		fmt.Println("after runlock")
+	}()
+	// ok: missing-runlock-on-rwmutex
+	return nil
+}
+
+func (c *RWContainer) inc6b(name string) error {
+	c.mu.RLock()
+	unlocker := c.mu.RUnlock
+	c.counters[name]++
+	defer func() {
+		fmt.Println("before runlock")
+		unlocker()
+		fmt.Println("after runlock")
+	}()
+	// todook: missing-runlock-on-rwmutex
+	return nil
+}
+
+func (c *RWContainer) inc7(name string) error {
+	c.mu.RLock()
+	c.counters[name]++
+	defer func(earlyExit bool) {
+		fmt.Println("before runlock")
+		if (earlyExit) {
+			// todoruleid: missing-runlock-on-rwmutex
+			return
+		}
+		c.mu.RUnlock()
+		fmt.Println("after runlock")
+	}(false)
+	// ok: missing-runlock-on-rwmutex
+	return nil
+}
+
+func (c *RWContainer) inc8(name string) error {
+	c.mu.RLock()
+	c.counters[name]++
+	_, err := fmt.Println("test if")
+	if err != nil {
+	    c.mu.RUnlock()
+	    // ok: missing-runlock-on-rwmutex
+	    return nil, err
+	}
+	// ruleid: missing-runlock-on-rwmutex
+	return nil
+}
+
+func (c *RWContainer) inc8b(name string) error {
+	c.mu.RLock()
+	c.counters[name]++
+	unlocker := c.mu.RUnlock
+	_, err := fmt.Println("test if")
+	if err != nil {
+	    unlocker()
+	    // todook: missing-runlock-on-rwmutex
+	    return nil, err
+	}
+	// ruleid: missing-runlock-on-rwmutex
+	return nil
+}

go/missing-runlock-on-rwmutex.yaml

@@ -1,7 +1,7 @@
 rules:
   - id: missing-runlock-on-rwmutex
     message: >-
-      Missing `RUnlock` on an `RWMutex` lock before returning from a function
+      Missing `RUnlock` on an `RWMutex` (`$T` variable) lock before returning from a function
     languages: [go]
     severity: ERROR
     metadata:
@@ -21,6 +21,11 @@ rules:
       - pattern-either:
           - pattern: panic(...)
           - pattern: return ...
+      - metavariable-pattern:
+          metavariable: $T
+          patterns:
+            - pattern: |
+                ($T : sync.RWMutex)
       - pattern-inside: |
           $T.RLock()
           ...
@@ -30,6 +35,13 @@ rules:
       - pattern-not-inside: |
           defer $T.RUnlock()
           ...
+      - pattern-not-inside: |
+          defer func(...) {
+            ...
+            $T.RUnlock()
+            ...
+          }(...)
+          ...
       - pattern-not-inside: |
           $FOO(..., ..., func(...) { 
               ... 

go/missing-unlock-before-return.go

@@ -14,6 +14,14 @@ type Container struct {
 	counters map[string]int
 }
 
+type Fridge struct {
+	food int
+}
+
+func (f *Fridge) Lock() {
+	fmt.Println(f.food)
+}
+
 func main() {
 	c := Container{
 		counters: map[string]int{"a": 0, "b": 0},
@@ -53,6 +61,7 @@ func (c *Container) inc(name string) error {
 		return fmt.Errorf("letter not allowed")
 	}
 	c.mu.Unlock()
+	// ok: missing-unlock-before-return
 	return nil
 }
 
@@ -65,6 +74,7 @@ func (c *Container) inc_FP(name string) error {
 		return fmt.Errorf("letter not allowed")
 	}
 	c.mu.Unlock()
+	// ok: missing-unlock-before-return
 	return nil
 }
 
@@ -78,6 +88,7 @@ func (c *Container) inc2(name string) error {
 		panic("letter not allowed")
 	}
 	c.mu.Unlock()
+	// ok: missing-unlock-before-return
 	return nil
 }
 
@@ -90,6 +101,7 @@ func (c *Container) inc2_FP(name string) error {
 		panic("letter not allowed")
 	}
 	c.mu.Unlock()
+	// ok: missing-unlock-before-return
 	return nil
 }
 
@@ -113,3 +125,78 @@ func (c *Container) inc4FP(name string) Unlocker {
 		c.mu.Unlock()
 	}
 }
+
+func (c *Container) inc5(name string) error {
+	f := Fridge{food: 11}
+	f.Lock()
+	// ok: missing-unlock-before-return
+	return nil
+}
+
+func (c *Container) inc6(name string) error {
+	c.mu.Lock()
+	c.counters[name]++
+	defer func() {
+		fmt.Println("before unlock")
+		c.mu.Unlock()
+		fmt.Println("after unlock")
+	}()
+	// ok: missing-unlock-before-return
+	return nil
+}
+
+func (c *Container) inc6b(name string) error {
+	c.mu.Lock()
+	unlocker := c.mu.Unlock
+	c.counters[name]++
+	defer func() {
+		fmt.Println("before unlock")
+		unlocker()
+		fmt.Println("after unlock")
+	}()
+	// todook: missing-unlock-before-return
+	return nil
+}
+
+func (c *Container) inc7(name string) error {
+	c.mu.Lock()
+	c.counters[name]++
+	defer func(earlyExit bool) {
+		fmt.Println("before unlock")
+		if (earlyExit) {
+			// todoruleid: missing-unlock-before-return
+			return
+		}
+		c.mu.Unlock()
+		fmt.Println("after unlock")
+	}(false)
+	// ok: missing-unlock-before-return
+	return nil
+}
+
+func (c *Container) inc8(name string) error {
+	c.mu.Lock()
+	c.counters[name]++
+	_, err := fmt.Println("test if")
+	if err != nil {
+	    c.mu.Unlock()
+	    // ok: missing-unlock-before-return
+	    return nil, err
+	}
+	// ruleid: missing-unlock-before-return
+	return nil
+}
+
+func (c *Container) inc8b(name string) error {
+	c.mu.Lock()
+	c.counters[name]++
+	unlocker := c.mu.Unlock
+	_, err := fmt.Println("test if")
+	if err != nil {
+	    unlocker()
+	    // todook: missing-unlock-before-return
+	    return nil, err
+	}
+	// ruleid: missing-unlock-before-return
+	return nil
+}

go/missing-unlock-before-return.yaml

@@ -1,7 +1,7 @@
 rules:
   - id: missing-unlock-before-return
     message: >-
-      Missing mutex unlock before returning from a function. 
+      Missing mutex unlock (`$T` variable) before returning from a function. 
       This could result in panics resulting from double lock operations
     languages: [go]
     severity: ERROR
@@ -22,6 +22,11 @@ rules:
       - pattern-either:
           - pattern: panic(...)
           - pattern: return ...
+      - metavariable-pattern:
+          metavariable: $T
+          patterns:
+            - pattern: |
+                ($T : sync.Mutex)
       - pattern-inside: |
           $T.Lock()
           ...
@@ -31,6 +36,13 @@ rules:
       - pattern-not-inside: |
           defer $T.Unlock()
           ...
+      - pattern-not-inside: |
+          defer func(...) {
+            ...
+            $T.Unlock()
+            ...
+          }(...)
+          ...
       - pattern-not-inside: |
           $FOO(..., ..., func(...) { 
               ...