Skip to content

Commit

Permalink
Add Semgrep Rules to OSS (#14513)
Browse files Browse the repository at this point in the history 
* add semgrep yml

* add semgrep ci job

* remove replication semgrep rule in oss

* fix makefile

* add semgrep to ci

* upwind triple if in ui.go semgrep refactoring
  • Loading branch information
Hridoy Roy authored Mar 18, 2022
1 parent 2fbaeca commit aaf3ce8
Show file tree
Hide file tree
Showing 29 changed files with 1,061 additions and 7 deletions.
25 changes: 25 additions & 0 deletions .circleci/config.yml
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions .circleci/config/commands/setup-semgrep.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
description: >
Ensure semgrep is installed.
steps:
- run:
working_directory: ~/
name: Setup Semgrep
command: |
apk add --no-cache python3 py3-pip make
python3 -m pip install --user semgrep
export PATH="$HOME/.local/bin:$PATH"
echo "$ semgrep --version"
semgrep --version
14 changes: 14 additions & 0 deletions .circleci/config/jobs/semgrep.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
executor: alpine
steps:
- setup-semgrep
- checkout
- attach_workspace:
at: .
- run:
name: Run Semgrep Rules
command: |
# Alpine images can't run the make file due to a bash requirement. Run
# semgrep explicitly here.
export PATH="$HOME/.local/bin:$PATH"
semgrep --error --include '*.go' --exclude 'vendor' -f tools/semgrep/ci .
3 changes: 3 additions & 0 deletions .circleci/config/workflows/ci.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,6 @@ jobs:
branches:
only:
- stable-website
- semgrep:
requires:
- pre-flight-checks
8 changes: 7 additions & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,12 @@ fmtcheck:
fmt:
find . -name '*.go' | grep -v pb.go | grep -v vendor | xargs gofumpt -w

semgrep:
semgrep --include '*.go' --exclude 'vendor' -a -f tools/semgrep .

semgrep-ci:
semgrep --error --include '*.go' --exclude 'vendor' -f tools/semgrep/ci .

assetcheck:
@echo "==> Checking compiled UI assets..."
@sh -c "'$(CURDIR)/scripts/assetcheck.sh'"
Expand Down Expand Up @@ -253,7 +259,7 @@ ci-config:
ci-verify:
@$(MAKE) -C .circleci ci-verify

.PHONY: bin default prep test vet bootstrap ci-bootstrap fmt fmtcheck mysql-database-plugin mysql-legacy-database-plugin cassandra-database-plugin influxdb-database-plugin postgresql-database-plugin mssql-database-plugin hana-database-plugin mongodb-database-plugin ember-dist ember-dist-dev static-dist static-dist-dev assetcheck check-vault-in-path check-browserstack-creds test-ui-browserstack packages build build-ci
.PHONY: bin default prep test vet bootstrap ci-bootstrap fmt fmtcheck mysql-database-plugin mysql-legacy-database-plugin cassandra-database-plugin influxdb-database-plugin postgresql-database-plugin mssql-database-plugin hana-database-plugin mongodb-database-plugin ember-dist ember-dist-dev static-dist static-dist-dev assetcheck check-vault-in-path check-browserstack-creds test-ui-browserstack packages build build-ci semgrep semgrep-ci

.NOTPARALLEL: ember-dist ember-dist-dev

Expand Down
20 changes: 20 additions & 0 deletions scripts/semgrep_plugin_repos.sh
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/bin/sh

set -e
set -x

## Make a temp dir
tempdir=$(mktemp -d plugin-semgrep.XXXXXX)
vaultdir=$(pwd)
## Set paths
cd $tempdir

for plugin in $(grep github.com/hashicorp/vault-plugin- $vaultdir/go.mod | cut -f 2 | cut -d ' ' -f 1 | cut -d '/' -f 3)
do
if [ -z $SKIP_MODULE_UPDATING ]
then
echo "Fetching $plugin..."
git clone https://github.com/hashicorp/$plugin
semgrep --include '*.go' --exclude 'vendor' -a -f $vaultdir/tools/semgrep/ci/ $plugin/. > $plugin.semgrep.txt
fi
done
17 changes: 17 additions & 0 deletions tools/semgrep/ci/atomic.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
rules:
- id: atomics-64bit-safety
patterns:
- pattern: |
type $TYPE struct {
...
$VAR atomic.$ATOMIC_TYPE
...
}
- metavariable-regex:
# We only care about 64 bit atomic types
metavariable: "$ATOMIC_TYPE"
regex: ".*64"
message: "Use pointers with member variables of uber-go/atomic types"
languages: [go]
severity: ERROR

14 changes: 14 additions & 0 deletions tools/semgrep/ci/bad-multierror-append.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
rules:
- id: bad-multierror-append
patterns:
- pattern-either:
- pattern: $ERR = multierror.Append($ERRORS, $ERR)
- pattern: $ERR = multierror.Append($ERR, $ERR)
- pattern: $ERRORS = multierror.Append($ERR, $ERR)
- pattern: $ERRORS = multierror.Append($ERR, $ERRORS)
message: Bad Multierror Append
languages:
- go
severity: ERROR
metadata:
license: MIT
17 changes: 17 additions & 0 deletions tools/semgrep/ci/bad-nil-guard.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
rules:
- id: bad-nil-guard
patterns:
- pattern-either:
- pattern: $X == nil && <... $X.$F ...>
- pattern: $X != nil || <... $X.$F ...>
- pattern: <... $X.$F ...> && $X != nil
- pattern: <... $X.$F ...> || $X == nil
- pattern: <... $X.$F ...> && $X == nil
- pattern: <... $X.$F ...> || $X != nil
message: Bad nil guard
languages:
- go
severity: ERROR
metadata:
license: MIT

123 changes: 123 additions & 0 deletions tools/semgrep/ci/error-shadowing.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
rules:
- id: error-shadow-check-types
patterns:
- pattern: |
..., ($ERR: error) = $FUNC(...)
...
..., $ERR = ...
- pattern-not: |
..., ($ERR: error) = $FUNC(...)
...
if <... $ERR == nil ...> {
...
}
...
..., $ERR = ...
- pattern-not: |
..., ($ERR: error) = $FUNC(...)
...
if <... $ERR != nil ...> {
...
}
...
..., $ERR = ...
- pattern-not: |
..., ($ERR: error) = $FUNC(...)
...
$ERRCHECK(..., $ERR, ...)
...
..., $ERR = ...
# This case is not specific enough but semgrep doesn't let you do any
# special searching within a switch statement. We will assume if there
# is a switch statement it's doing error checking, though this isn't
# guaranteed.
- pattern-not: |
..., ($ERR: error) = $FUNC(...)
...
switch {
case ...
}
...
..., $ERR = ...
message: Potential Error Shadowing
languages:
- go
severity: ERROR


- id: error-shadow-check-regex
patterns:
- pattern: |
..., $ERR = $FUNC(...)
...
..., $ERR = ...
- pattern-not: |
..., $ERR = $FUNC(...)
...
if <... $ERR == nil ...> {
...
}
...
..., $ERR = ...
- pattern-not: |
..., $ERR = $FUNC(...)
...
if <... $ERR != nil ...> {
...
}
...
..., $ERR = ...
- pattern-not: |
..., $ERR = $FUNC(...)
...
$ERRCHECK(..., $ERR, ...)
...
..., $ERR = ...
# This pattern is used in as a itteration mechanism for a test
- pattern-not: |
..., $ERR = $FUNC(...)
...
for $ERR == nil {
...
}
...
..., $ERR = ...
# A few places we test against logical.Err* types
- pattern-not: |
..., $ERR = $FUNC(...)
...
if $ERR != logical.$ERRTYPE {
...
}
...
..., $ERR = ...
# This case is not specific enough but semgrep doesn't let you do any
# special searching within a switch statement. We will assume if there
# is a switch statement it's doing error checking, though this isn't
# guaranteed.
- pattern-not: |
..., $ERR = $FUNC(...)
...
switch ... {
case ...
}
...
..., $ERR = ...
- pattern-not: |
..., $ERR = $FUNC(...)
...
switch {
case ...
}
...
..., $ERR = ...
- metavariable-regex:
metavariable: $ERR
regex: "err"
message: Potential Error Shadowing (regex)
languages:
- go
severity: ERROR

25 changes: 25 additions & 0 deletions tools/semgrep/ci/hashsum.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
rules:
- id: hash-sum-without-write
patterns:
- pattern-either:
- pattern: |
$HASH.New().Sum($SLICE)
- pattern: |
$H := $HASH.New()
...
$H.Sum($SLICE)
- pattern-not: |
$H := $HASH.New()
...
$H.Write(...)
...
$H.Sum($SLICE)
- pattern-not: |
$H := $HASH.New()
...
$FUNC(..., $H, ...)
...
$H.Sum($SLICE)
message: "odd hash.Sum call flow"
languages: [go]
severity: ERROR
19 changes: 19 additions & 0 deletions tools/semgrep/ci/hmac-bytes.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
rules:
- id: use-hmac-equal
patterns:
- pattern-either:
- pattern: |
$MAC = hmac.New(...)
...
$H = $MAC.Sum(...)
...
bytes.Equal($H, ...)
- pattern: |
$MAC = hmac.New(...)
...
$H = $MAC.Sum(...)
...
bytes.Equal(..., $H)
message: "Comparing a MAC with bytes.Equal()"
languages: [go]
severity: ERROR
21 changes: 21 additions & 0 deletions tools/semgrep/ci/hmac-hash.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
rules:
- id: hmac-needs-new
patterns:
- pattern-either:
- pattern: |
$H := $HASH.New()
...
$FUNC := func() hash.Hash { return $H }
...
hmac.New($FUNC, ...)
- pattern: |
$H := $HASH.New()
...
hmac.New(func() hash.Hash { return $H }, ...)
- pattern: |
hmac.New(func() hash.Hash { return ( $H : hash.Hash) }, ...)
message: "calling hmac.New with unchanging hash.New"
languages: [go]
severity: ERROR
22 changes: 22 additions & 0 deletions tools/semgrep/ci/logger-format-string.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
rules:
- id: logger-used-with-format-string
patterns:
- pattern-either:
- pattern: |
$LOGGER.Trace("=~/.*%[v#T%tbcdoOqxXUbeEfFgGps].*/",...)
- pattern: |
$LOGGER.Debug("=~/.*%[v#T%tbcdoOqxXUbeEfFgGps].*/",...)
- pattern: |
$LOGGER.Info("=~/.*%[v#T%tbcdoOqxXUbeEfFgGps].*/",...)
- pattern: |
$LOGGER.Warn("=~/.*%[v#T%tbcdoOqxXUbeEfFgGps].*/",...)
- pattern: |
$LOGGER.Error("=~/.*%[v#T%tbcdoOqxXUbeEfFgGps].*/",...)
- pattern-inside: |
import $LOG "github.com/hashicorp/go-hclog"
...
message: "Logger message looks like format string"
languages: [go]
severity: ERROR


Loading

0 comments on commit aaf3ce8

Please sign in to comment.