Skip to content

Commit 8ed2c85

Browse files
committed
Merge branch 'codeql'
This patch series has been long in the making, ever since Johannes Nicolai and myself spiked this in November/December 2020. Signed-off-by: Johannes Schindelin <[email protected]>
2 parents 42e298f + e33df9a commit 8ed2c85

File tree

4 files changed

+198
-1
lines changed

4 files changed

+198
-1
lines changed

.github/codeql/codeql-config.yml

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
name: "CodeQL config"
2+
3+
queries:
4+
- uses: security-extended
5+
6+
query-filters:
7+
- exclude:
8+
# yes, this extra indentation is intentional
9+
# too common in Git's source code
10+
id: cpp/trivial-switch
11+
- exclude:
12+
id: cpp/loop-variable-changed
13+
- exclude:
14+
# we override this locally with a modified version
15+
id: cpp/non-constant-format
16+
- exclude:
17+
# Git does not consider this a problem
18+
id: cpp/irregular-enum-init
19+
- exclude:
20+
# Git has many long functions, this alert would match too many
21+
id: cpp/poorly-documented-function
22+
- exclude:
23+
# In Git, there is a lot of commented-out code
24+
id: cpp/commented-out-code
25+
- exclude:
26+
# While it is true that long switch cases are hard to read and
27+
# validate, Git has way too many for us to allow this query to
28+
# churn out alerts left and right
29+
id: cpp/long-switch
30+
- exclude:
31+
# CodeQL does not expect Git to heed the umask(), but it does
32+
id: cpp/world-writable-file-creation
33+
- exclude:
34+
# Git uses the construct `if (<not this>) ; else ...` often, to
35+
# avoid an extra indentation level. CodeQL does not like that.
36+
id: cpp/empty-block
37+
- exclude:
38+
# This rule unfortunately triggers some false positives, e.g.
39+
# where Git tries to redact URLs or where Git specifically
40+
# asks for a password upon GIT_SSL_CERT_PASSWORD_PROTECTED.
41+
id: cpp/user-controlled-bypass
42+
- exclude:
43+
# This rule fails to recognize that xmallocz() _specifically_
44+
# makes room for a trailing NUL, and instead assumes that this
45+
# function behaves like malloc(), which does not.
46+
id: cpp/invalid-pointer-deref
47+
- exclude:
48+
# CodeQL fails to recognize that xmallocz() accounts for the NUL,
49+
# instead assuming malloc() semantics.
50+
id: cpp/no-space-for-terminator
51+
- exclude:
52+
# Git does exchange plain-text passwords via stdin/stdout e.g.
53+
# with helpers in the credential protocol, or in credential-cache.
54+
# This rule, though, assumes that writing to _any_ file descriptor
55+
# is unsafe.
56+
id: cpp/cleartext-storage-file
57+
- exclude:
58+
# When storing the value of the environment variable `PWD` as the
59+
# current directory in absolute_pathdup(), or when allocating memory
60+
# for a binary patch where the size is specified in the patch itself,
61+
# CodeQL assumes that this can lead to a denial of service because
62+
# of an unbounded size, but Git's code works as designed here.
63+
id: cpp/uncontrolled-allocation-size
64+
- exclude:
65+
# lock_repo_for_gc() has admittedly obtuse logic to parse the
66+
# process ID out of the `gc.pid` file, which is correct, but
67+
# due to its construction throws a false positive here.
68+
id: cpp/missing-check-scanf
69+
- exclude:
70+
# discard_cache_entry() overwrites the name in a FLEX_ARRAY struct
71+
# if GIT_TEST_VALIDATE_INDEX_CACHE_ENTRIES is set, which CodeQL fails
72+
# to recognize as valid.
73+
id: cpp/overrun-write
74+
- exclude:
75+
# Since `time_t` can be signed or unsigned, there is unfortunately
76+
# no way to avoid letting this rule report a potential
77+
id: cpp/integer-multiplication-cast-to-long
78+
- exclude:
79+
# There are many, many legitimate code paths in Git where a path is
80+
# constructed from an environment variable, e.g. GIT_DIR. Let's suppress
81+
# this slightly overzealous query.
82+
id: cpp/path-injection
83+
- exclude:
84+
# Git has 99 instances of this at the time of writing :-(
85+
id: cpp/declaration-hides-variable
86+
- exclude:
87+
id: cpp/declaration-hides-parameter
88+
- exclude:
89+
id: cpp/local-variable-hides-global-variable
90+
- exclude:
91+
id: cpp/complex-condition
92+
- exclude:
93+
# Nested, long-winded switch statements are hard to read and hard
94+
# to reason about. Looking at you, `format_commit_one()`.
95+
id: cpp/complex-block
96+
- exclude:
97+
# There are four instances of this at time of writing, all intentional.
98+
# However, it is very easy to introduce unintentional re-use of loop
99+
# variable names, therefore we will most likely want to either change these
100+
# instances or add suppressions.
101+
id: cpp/nested-loops-with-same-variable
102+
- exclude:
103+
# zOMG so many FIXMEs
104+
id: cpp/fixme-comment
105+
- exclude:
106+
# Git assumes quite a bit about the user's control of the current worktree
107+
# Therefore, it kind of assumes that TOCTOU issues are not a thing when
108+
# it comes to files.
109+
id: cpp/toctou-race-condition
110+
- exclude:
111+
# Too many results in Git where the code was, however, intentionally written
112+
# the way it is.
113+
id: cpp/stack-address-escape
114+
- exclude:
115+
id: cpp/inconsistent-null-check
116+
- exclude:
117+
# This would trigger alerts in the functions in `help.c` that want to open
118+
# external programs to show manual pages.
119+
id: cpp/uncontrolled-process-operation
120+
- exclude:
121+
# The code in t/unit-tests/u-ctype.c implicitly exercises the `sane_istest()`
122+
# macro extensively, and CodeQL seems to miss the cast to `(unsigned char)`,
123+
# thereby mistaking the accesses for being past the end of the array (which
124+
# is incorrect).
125+
#
126+
# Ideally, we would exclude test programs from CodeQL anyways, but
127+
# unfortunately there is no Makefile rule in Git's code base to build only
128+
# the production code, and CodeQL's `paths-ignore` directive described at
129+
# https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning#specifying-directories-to-scan
130+
# unfortunately is _ignored_ for compiled languages.
131+
id: cpp/overflow-buffer

.github/workflows/codeql.yml

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
name: "CodeQL"
2+
3+
on:
4+
push:
5+
pull_request:
6+
workflow_dispatch:
7+
8+
jobs:
9+
analyze:
10+
name: Analyze
11+
runs-on: ubuntu-latest
12+
permissions:
13+
actions: read
14+
contents: read
15+
security-events: write
16+
17+
strategy:
18+
fail-fast: false
19+
matrix:
20+
language: ["cpp"]
21+
22+
steps:
23+
- name: Checkout repository
24+
uses: actions/checkout@v3
25+
26+
- name: Install dependencies
27+
run: ci/install-dependencies.sh
28+
if: matrix.language == 'cpp'
29+
env:
30+
jobname: codeql
31+
32+
# Initializes the CodeQL tools for scanning.
33+
- name: Initialize CodeQL
34+
uses: github/codeql-action/init@v3
35+
with:
36+
languages: ${{ matrix.language }}
37+
config-file: ./.github/codeql/codeql-config.yml
38+
39+
- name: Build
40+
if: matrix.language == 'cpp'
41+
run: |
42+
cat /proc/cpuinfo
43+
make -j$(nproc)
44+
45+
- name: Perform CodeQL Analysis
46+
uses: github/codeql-action/analyze@v3
47+
with:
48+
upload: False
49+
output: sarif-results
50+
51+
- name: debug
52+
shell: bash
53+
run: ls -la sarif-results
54+
55+
- name: publish sarif for debugging
56+
uses: actions/upload-artifact@v4
57+
with:
58+
name: sarif-results
59+
path: sarif-results
60+
61+
- name: Upload SARIF
62+
uses: github/codeql-action/upload-sarif@v3
63+
with:
64+
sarif_file: sarif-results/cpp.sarif

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,3 +254,5 @@ Release/
254254
/contrib/buildsystems/out
255255
/contrib/libgit-rs/target
256256
/contrib/libgit-sys/target
257+
/.github/codeql/.cache/
258+
/.github/codeql/codeql-pack.lock.yml

ci/install-dependencies.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ ClangFormat)
119119
sudo apt-get -q update
120120
sudo apt-get -q -y install clang-format
121121
;;
122-
StaticAnalysis)
122+
StaticAnalysis|codeql)
123123
sudo apt-get -q update
124124
sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \
125125
libexpat-dev gettext make

0 commit comments

Comments
 (0)