From a52e6e87ecfd010c57fd7c061a2b1ba87a76503d Mon Sep 17 00:00:00 2001 From: Philippe Coval Date: Tue, 20 May 2025 17:08:54 +0200 Subject: [PATCH 1/5] zwave_rx: Harden zwave_rx_print_protocol_version in zwave_rx.c Checking snprintf results, this was found using CodeQL Potential fix for code scanning alert no. 15: Potentially overflowing call to snprintf For the record this function escape the git commit to hex form (in ascii) Origin: https://github.com/SiliconLabsSoftware/z-wave-protocol-controller/pull/104 Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Relate-to: https://github.com/SiliconLabsSoftware/z-wave-protocol-controller/issues/100 Signed-off-by: Philippe Coval --- .../components/zwave/zwave_rx/src/zwave_rx.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/applications/zpc/components/zwave/zwave_rx/src/zwave_rx.c b/applications/zpc/components/zwave/zwave_rx/src/zwave_rx.c index b5e11e97a..24e376aea 100644 --- a/applications/zpc/components/zwave/zwave_rx/src/zwave_rx.c +++ b/applications/zpc/components/zwave/zwave_rx/src/zwave_rx.c @@ -11,6 +11,7 @@ * *****************************************************************************/ //Generic includes +#include #include // Includes from other components @@ -89,11 +90,18 @@ static void zwave_rx_print_protocol_version( char git_commit_string[GIT_COMMIT_HASH_SIZE * 2 + 1] = {0}; uint16_t index = 0; for (uint8_t i = 0; i < GIT_COMMIT_HASH_SIZE; i++) { - index += snprintf(git_commit_string + index, - sizeof(git_commit_string) - index, - "%x", - zwapi_version.git_commit[i]); - } + int written = snprintf(git_commit_string + index, + sizeof(git_commit_string) - index, + "%x", + zwapi_version.git_commit[i]); + if (written < 0 || written >= (int)(sizeof(git_commit_string) - index)) { + sl_log_error(LOG_TAG, "Error in zwave_rx_print_protocol_version"); + assert(false); + // Stop processing if snprintf fails or would overflow the buffer + break; + } + index += written; + } sl_log_info(LOG_TAG, "Z-Wave API protocol git commit: %s\n", From 7830c464771de990bb0376dac40add73e2a3722f Mon Sep 17 00:00:00 2001 From: Philippe Coval Date: Thu, 22 May 2025 16:11:24 +0200 Subject: [PATCH 2/5] ci: github: Remove use of token This will align to SL policy Signed-off-by: Philippe Coval --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 78c1f761a..42ac28bdc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,6 +17,7 @@ on: # yamllint disable-line rule:truthy jobs: test: permissions: + actions: read contents: read statuses: write env: @@ -30,7 +31,6 @@ jobs: with: image: "${{ env.project-name }}:latest" workflow: "build" - token: ${{ secrets.GH_SL_ACCESS_TOKEN }} workflow_run_id: ${{ github.event.workflow_run.id }} # yamllint disable-line rule:line-length From cf0f068b2cbeabf753c4190052344aae2a5861c7 Mon Sep 17 00:00:00 2001 From: Philippe Coval Date: Mon, 19 May 2025 12:28:30 +0200 Subject: [PATCH 3/5] ci: build: Enable build on PR and prevent use of pull_request_target Also added comment to prevent privileges escalation using pull_request_target (see related change) Relate-to:https://github.com/SiliconLabsSoftware/z-wave-protocol-controller/issues/67 Relate-to: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/ Signed-off-by: Philippe Coval --- .github/workflows/build-rootfs.yml | 4 ++++ .github/workflows/build.yml | 5 +++++ .github/workflows/test.yml | 4 ++++ 3 files changed, 13 insertions(+) diff --git a/.github/workflows/build-rootfs.yml b/.github/workflows/build-rootfs.yml index a48806909..ba504b6ee 100644 --- a/.github/workflows/build-rootfs.yml +++ b/.github/workflows/build-rootfs.yml @@ -3,6 +3,7 @@ name: z-wave-protocol-controller Build in rootfs for arch on: # yamllint disable-line rule:truthy + # pull_request_target: # Avoid to prevent CodeQL CWE-829 push: tags: - '*' @@ -18,6 +19,9 @@ jobs: - arm64 # - armhf # TODO Enable when supported steps: + - name: Security check + if: ${{ github.event.action == 'pull_request_target'}} + run: echo "Prevent running (CodeQL CWE-829)" && exit 1 # yamllint disable-line rule:line-length - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7326fd085..c77171c9c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -6,6 +6,8 @@ name: build on: # yamllint disable-line rule:truthy + pull_request: + # pull_request_target: # Avoid to prevent CodeQL CWE-829 push: jobs: @@ -16,6 +18,9 @@ jobs: project-name: z-wave-protocol-controller # Align to docker (lowercase) runs-on: ubuntu-22.04 steps: + - name: Security check + if: ${{ github.event.action == 'pull_request_target'}} + run: echo "Prevent running (CodeQL CWE-829)" && exit 1 # yamllint disable-line rule:line-length - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 42ac28bdc..b6cfcc85d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -9,6 +9,7 @@ name: test run-name: "test: ${{ github.event.workflow_run.head_branch }}#${{ github.event.workflow_run.head_commit.id }}" on: # yamllint disable-line rule:truthy + # pull_request_target: # Avoid to prevent CodeQL CWE-829 workflow_run: workflows: ["build"] types: @@ -25,6 +26,9 @@ jobs: runs-on: ubuntu-24.04 if: ${{ github.event.workflow_run.conclusion == 'success' }} steps: + - name: Security check + if: ${{ github.event.action == 'pull_request_target'}} + run: echo "Prevent running (CodeQL CWE-829)" && exit 1 - name: Download image # yamllint disable-line rule:line-length uses: ishworkh/container-image-artifact-download@ccb3671db007622e886a2d7037eb62b119d5ffaf # v2.0.0 From 87bd8293c307010afc73082bca913e66e312b43b Mon Sep 17 00:00:00 2001 From: Phil Coval Date: Thu, 22 May 2025 17:32:44 +0200 Subject: [PATCH 4/5] Potential fix for code scanning alert no. 6: Potentially overflowing call to snprintf Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../src/zwave_api_demo_callbacks.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/applications/zpc/applications/zwave_api_demo/src/zwave_api_demo_callbacks.c b/applications/zpc/applications/zwave_api_demo/src/zwave_api_demo_callbacks.c index 7ca6934c6..eaf3038f4 100644 --- a/applications/zpc/applications/zwave_api_demo/src/zwave_api_demo_callbacks.c +++ b/applications/zpc/applications/zwave_api_demo/src/zwave_api_demo_callbacks.c @@ -305,12 +305,21 @@ void zwapi_demo_zwave_api_started(const uint8_t *buffer, uint8_t buffer_length) char message[MAXIMUM_MESSAGE_SIZE]; uint8_t index = 0; - index += snprintf(message + index, - sizeof(message) - index, - "Z-Wave API started. Current NIF: "); + int n = snprintf(message + index, + sizeof(message) - index, + "Z-Wave API started. Current NIF: "); + if (n < 0 || n >= (int)(sizeof(message) - index)) { + sl_log_error(LOG_TAG, "Buffer overflow prevented while writing message."); + return; + } + index += n; for (uint8_t i = 0; i < buffer_length; i++) { - index - += snprintf(message + index, sizeof(message) - index, "%02X ", buffer[i]); + n = snprintf(message + index, sizeof(message) - index, "%02X ", buffer[i]); + if (n < 0 || n >= (int)(sizeof(message) - index)) { + sl_log_error(LOG_TAG, "Buffer overflow prevented while writing message."); + return; + } + index += n; } sl_log_info(LOG_TAG, "%s\n", message); } From 8c3b5dbd4ba94c9f6a439feb090754577ad506aa Mon Sep 17 00:00:00 2001 From: Phil Coval Date: Thu, 22 May 2025 17:39:19 +0200 Subject: [PATCH 5/5] Potential fix for code scanning alert no. 19: Potentially overflowing call to snprintf Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../zwave_api/src/zwapi_connection.c | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/applications/zpc/components/zwave_api/src/zwapi_connection.c b/applications/zpc/components/zwave_api/src/zwapi_connection.c index b9be78e1f..883574010 100644 --- a/applications/zpc/components/zwave_api/src/zwapi_connection.c +++ b/applications/zpc/components/zwave_api/src/zwapi_connection.c @@ -50,14 +50,37 @@ static const char *zwapi_frame_to_string(const uint8_t *buffer, // Don't log the SOF byte. continue; } else if (i == 1) { - index += snprintf(message + index, sizeof(message) - index, "Length="); + { + int n = snprintf(message + index, sizeof(message) - index, "Length="); + if (n < 0 || n >= sizeof(message) - index) { + break; + } + index += n; + } } else if (i == 2) { - index += snprintf(message + index, sizeof(message) - index, "Type="); + { + int n = snprintf(message + index, sizeof(message) - index, "Type="); + if (n < 0 || n >= sizeof(message) - index) { + break; + } + index += n; + } } else if (i == 3) { - index += snprintf(message + index, sizeof(message) - index, "Cmd="); + { + int n = snprintf(message + index, sizeof(message) - index, "Cmd="); + if (n < 0 || n >= sizeof(message) - index) { + break; + } + index += n; + } + } + { + int n = snprintf(message + index, sizeof(message) - index, "%02X ", buffer[i]); + if (n < 0 || n >= sizeof(message) - index) { + break; + } + index += n; } - index - += snprintf(message + index, sizeof(message) - index, "%02X ", buffer[i]); } return message; }