From 2b0ef5c330b457447e2e2331550f11272d0158da Mon Sep 17 00:00:00 2001 From: weichslgartner Date: Sun, 14 Nov 2021 13:47:04 +0100 Subject: [PATCH 1/4] Create codeql-analysis.yml --- .github/workflows/codeql-analysis.yml | 70 +++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 .github/workflows/codeql-analysis.yml diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml new file mode 100644 index 0000000..97daee1 --- /dev/null +++ b/.github/workflows/codeql-analysis.yml @@ -0,0 +1,70 @@ +# For most projects, this workflow file will not need changing; you simply need +# to commit it to your repository. +# +# You may wish to alter this file to override the set of languages analyzed, +# or to provide custom queries or build logic. +# +# ******** NOTE ******** +# We have attempted to detect the languages in your repository. Please check +# the `language` matrix defined below to confirm you have the correct set of +# supported CodeQL languages. +# +name: "CodeQL" + +on: + push: + branches: [ master ] + pull_request: + # The branches below must be a subset of the branches above + branches: [ master ] + schedule: + - cron: '29 21 * * 1' + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write + + strategy: + fail-fast: false + matrix: + language: [ 'cpp' ] + # CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python', 'ruby' ] + # Learn more about CodeQL language support at https://git.io/codeql-language-support + + steps: + - name: Checkout repository + uses: actions/checkout@v2 + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + with: + languages: ${{ matrix.language }} + # If you wish to specify custom queries, you can do so here or in a config file. + # By default, queries listed here will override any specified in a config file. + # Prefix the list here with "+" to use these queries and those in the config file. + # queries: ./path/to/local/query, your-org/your-repo/queries@main + + # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). + # If this step fails, then you should remove it and run the build manually (see below) + - name: Autobuild + uses: github/codeql-action/autobuild@v1 + + # ℹī¸ Command-line programs to run using the OS shell. + # 📚 https://git.io/JvXDl + + # ✏ī¸ If the Autobuild fails above, remove it and uncomment the following three lines + # and modify them (or add more) to build your code if your project + # uses a compiled language + + #- run: | + # make bootstrap + # make release + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 From 34732eddeb7a4bcf2f748d7a94eb6a806f370e88 Mon Sep 17 00:00:00 2001 From: weichslgartner Date: Mon, 13 Jun 2022 15:51:31 +0200 Subject: [PATCH 2/4] fixed signed integer overflow in asc2log.c:100 Issue: echo "0.0000 0 Rx d 8 8D 00 10 01 00 82 01 00 0.200000000000000000 0- 0000 Rx d 8 8D 00 10 01 00 82 01 00" | ./asc2log can-utils/asc2log.c:100:20: runtime error: signed integer overflow: 200000000000000000 * 100 cannot be represented in type 'long' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior asc2log.c:100:20 --- asc2log.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/asc2log.c b/asc2log.c index 7f932ef..ea6b486 100644 --- a/asc2log.c +++ b/asc2log.c @@ -50,6 +50,7 @@ #include #include #include +#include #include #include @@ -206,6 +207,13 @@ void eval_can(char* buf, struct timeval *date_tvp, char timestamps, char base, i if (strlen(dir) != 2) /* "Rx" or "Tx" */ return; + /* check for signed integer overflow */ + if (dplace == 4 && read_tv.tv_usec >= INT_MAX / 100) + return; + + if (dplace == 5 && read_tv.tv_usec >= INT_MAX / 10) + return; + if (dir[0] == 'R') extra_info = " R\n"; else @@ -269,6 +277,14 @@ void eval_canfd(char* buf, struct timeval *date_tvp, char timestamps, int dplace if (strlen(dir) != 2) /* "Rx" or "Tx" */ return; + /* check for signed integer overflow */ + if (dplace == 4 && read_tv.tv_usec >= INT_MAX / 100) + return; + + /* check for signed integer overflow */ + if (dplace == 5 && read_tv.tv_usec >= INT_MAX / 10) + return; + if (dir[0] == 'R') extra_info = " R\n"; else From eb2b38790a157f4a4d3c9b86cf0b5122d717b74b Mon Sep 17 00:00:00 2001 From: weichslgartner Date: Mon, 13 Jun 2022 15:53:55 +0200 Subject: [PATCH 3/4] fixes undefined behavior in parse_canframe (lib.c:187) by chaning tmp to canid_t Issue: mkdir build && cd build CC=clang cmake -DCMAKE_C_FLAGS="-fsanitize=address,undefined" -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined" .. && cmake --build . echo "(0.0) can1 ffffffff#00000000" | ./log2long can-utils/lib.c:187:23: runtime error: left shift of 15 by 28 places cannot be represented in type 'int' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /can-utils/lib.c:187:23 in (0.0) can1 3FFFFFFF --- lib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib.c b/lib.c index 935270b..f0f099e 100644 --- a/lib.c +++ b/lib.c @@ -159,7 +159,7 @@ int parse_canframe(char *cs, struct canfd_frame *cf) { int i, idx, dlen, len; int maxdlen = CAN_MAX_DLEN; int ret = CAN_MTU; - unsigned char tmp; + canid_t tmp; len = strlen(cs); //printf("'%s' len %d\n", cs, len); From ef853f555379babc121b90af4e8d2427aef7f2d3 Mon Sep 17 00:00:00 2001 From: weichslgartner Date: Mon, 20 Jun 2022 19:22:11 +0200 Subject: [PATCH 4/4] added return value check of snprintf to prevent possible buffer overflows detected by CodeQL --- lib.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/lib.c b/lib.c index f0f099e..81fe64b 100644 --- a/lib.c +++ b/lib.c @@ -568,10 +568,20 @@ static int snprintf_error_data(char *buf, size_t len, uint8_t err, for (i = 0; i < arr_len; i++) { if (err & (1 << i)) { - if (count) - n += snprintf(buf + n, len - n, ","); - n += snprintf(buf + n, len - n, "%s", arr[i]); - count++; + int tmp_n = 0; + if (count){ + /* Fix for potential buffer overflow https://lgtm.com/rules/1505913226124/ */ + tmp_n = snprintf(buf + n, len - n, ","); + if (tmp_n < 0 || tmp_n >= len - n){ + return n; + } + n += tmp_n; + } + tmp_n = snprintf(buf + n, len - n, "%s", arr[i]); + if (tmp_n < 0 || tmp_n >= len - n){ + return n; + } + n += tmp_n; } } @@ -644,9 +654,20 @@ void snprintf_can_error_frame(char *buf, size_t len, const struct canfd_frame *c for (i = 0; i < (int)ARRAY_SIZE(error_classes); i++) { mask = 1 << i; if (class & mask) { - if (classes) - n += snprintf(buf + n, len - n, "%s", sep); - n += snprintf(buf + n, len - n, "%s", error_classes[i]); + int tmp_n = 0; + if (classes){ + /* Fix for potential buffer overflow https://lgtm.com/rules/1505913226124/ */ + tmp_n = snprintf(buf + n, len - n, "%s", sep); + if (tmp_n < 0 || tmp_n >= len - n){ + return; + } + n += tmp_n; + } + tmp_n = snprintf(buf + n, len - n, "%s", error_classes[i]); + if (tmp_n < 0 || tmp_n >= len - n){ + return; + } + n += tmp_n; if (mask == CAN_ERR_LOSTARB) n += snprintf_error_lostarb(buf + n, len - n, cf);