Skip to content

Commit d6716da

Browse files
Add semgrep rules for wrong error return (#341)
* Add semgrep rule 'if-inplace-func-incorrect-nil-err-return'. * Add experimental semgrep rule 'if-incorrect-nil-err-return'. Put in experimental directory because of huge count of false positives. * Add '.semgrep/rules' config to semgrep CI job. * Semgrep rule 'if-incorrect-nil-err-return' moved to regular rules. --------- Co-authored-by: Alexey Kiselev <[email protected]>
1 parent 277caf8 commit d6716da

File tree

3 files changed

+95
-1
lines changed

3 files changed

+95
-1
lines changed

.github/workflows/security.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ jobs:
4747
semgrep ci --config="auto" --config="r/default" --config="r/go" --config="r/dgryski" \
4848
--config="r/trailofbits" --config="r/dockerfile" --config="r/bash" \
4949
--config="r/problem-based-packs" --config="r/generic" --config="r/yaml" --config="r/json" \
50-
--sarif --dataflow-traces --output=semgrep.sarif --max-target-bytes=2MB
50+
--config="./.semgrep/rules" --sarif --dataflow-traces --output=semgrep.sarif --max-target-bytes=2MB
5151
EXIT_CODE=$?
5252
if [ "$EXIT_CODE" = "0" ] || [ "$EXIT_CODE" = "1" ]
5353
then
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
rules:
2+
- id: if-incorrect-nil-err-return
3+
languages: [go]
4+
severity: WARNING
5+
message: |
6+
WARNING: A local variable '$ERR' is checked for nil, but a different variable is returned.
7+
Ensure that the returned variable is the one that was checked or properly wrapped!
8+
patterns:
9+
- metavariable-regex:
10+
metavariable: $ERR
11+
regex: .*(?i)err # using .* to allow prefixes, because regex matching is left anchored.
12+
13+
- pattern: |
14+
if $ERR != nil {
15+
...
16+
return ..., $OTHERERR
17+
}
18+
19+
- pattern-not: |
20+
if $ERR != nil {
21+
...
22+
return ..., $ERR
23+
}
24+
- pattern-not: |
25+
if $ERR != nil {
26+
...
27+
return ..., $ANYFUNC(..., $ERR, ...)
28+
}
29+
- pattern-not: |
30+
if $ERR != nil {
31+
...
32+
return ..., $ANYFUNC(..., $ANYFUNC1(..., $ERR, ...), ...)
33+
}
34+
- pattern-not: |
35+
if $ERR != nil {
36+
...
37+
$NEWERR := $ANYFUNC(..., $ERR, ...)
38+
...
39+
return nil, $NEWERR
40+
}
41+
- pattern-not: |
42+
if $ERR != nil {
43+
...
44+
$NEWERR := $ANYFUNC(..., $ERR, ...)
45+
...
46+
return ..., $ANYFUNC1(..., $NEWERR, ...)
47+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
rules:
2+
- id: if-inplace-func-incorrect-nil-err-return
3+
languages: [go]
4+
severity: WARNING
5+
message: |
6+
WARNING: A local variable '$ERR' is checked for nil, but a different variable is returned.
7+
Ensure that the returned variable is the one that was checked or properly wrapped!
8+
patterns:
9+
- metavariable-regex:
10+
metavariable: $ERR
11+
regex: .*(?i)err # using .* to allow prefixes, because regex matching is left anchored.
12+
13+
- pattern: |
14+
if $ERR := $FUNC(...); $ERR != nil {
15+
...
16+
return ..., $OTHERERR
17+
}
18+
19+
- pattern-not: |
20+
if $ERR := $FUNC(...); $ERR != nil {
21+
...
22+
return ..., $ERR
23+
}
24+
- pattern-not: |
25+
if $ERR := $FUNC(...); $ERR != nil {
26+
...
27+
return ..., $ANYFUNC(..., $ERR, ...)
28+
}
29+
- pattern-not: |
30+
if $ERR := $FUNC(...); $ERR != nil {
31+
...
32+
return ..., $ANYFUNC(..., $ANYFUNC1(..., $ERR, ...), ...)
33+
}
34+
- pattern-not: |
35+
if $ERR := $FUNC(...); $ERR != nil {
36+
...
37+
$NEWERR := $ANYFUNC(..., $ERR, ...)
38+
...
39+
return nil, $NEWERR
40+
}
41+
- pattern-not: |
42+
if $ERR := $FUNC(...); $ERR != nil {
43+
...
44+
$NEWERR := $ANYFUNC(..., $ERR, ...)
45+
...
46+
return ..., $ANYFUNC1(..., $NEWERR, ...)
47+
}

0 commit comments

Comments
 (0)