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 843c9b1686baa91d1d27de55a42eec59d31ecba4 Mon Sep 17 00:00:00 2001 From: Phil Coval Date: Thu, 22 May 2025 17:37:00 +0200 Subject: [PATCH 5/5] Potential fix for code scanning alert no. 12: Potentially overflowing call to snprintf Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- applications/zpc/components/zpc_utils/src/zpc_converters.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/applications/zpc/components/zpc_utils/src/zpc_converters.c b/applications/zpc/components/zpc_utils/src/zpc_converters.c index 5e174c701..990809e10 100644 --- a/applications/zpc/components/zpc_utils/src/zpc_converters.c +++ b/applications/zpc/components/zpc_utils/src/zpc_converters.c @@ -52,7 +52,11 @@ sl_status_t zpc_converters_dsk_to_str(const zwave_dsk_t src, size_t index = 0; for (int i = 0; i < sizeof(zwave_dsk_t); i += 2) { int d = (src[i] << 8) | src[i + 1]; - index += snprintf(&dst[index], dst_max_len - index, "%05i-", d); + int n = snprintf(&dst[index], dst_max_len - index, "%05i-", d); + if (n < 0 || n >= dst_max_len - index) { + return SL_STATUS_WOULD_OVERFLOW; + } + index += n; } // Erase the last "-" if (index > 0) {