Skip to content

[lts_03_2025] Use safe_add_size_t when sizing DPS transport response buffers (backport of #2738) - #2746

Open
Ewerton Scaboro da Silva (ewertons) wants to merge 2 commits into
lts_03_2025from
backport/lts_03_2025-dps-safe-add
Open

[lts_03_2025] Use safe_add_size_t when sizing DPS transport response buffers (backport of #2738)#2746
Ewerton Scaboro da Silva (ewertons) wants to merge 2 commits into
lts_03_2025from
backport/lts_03_2025-dps-safe-add

Conversation

@ewertons

@ewertons Ewerton Scaboro da Silva (ewertons) commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Backports #2738 to lts_03_2025, the only LTS branch still in maintenance (end date 2026-10-07 per the LTS schedule in readme.md).

Why

The DPS transports size their response buffers with malloc(len + 1), so a len of SIZE_MAX would wrap to malloc(0) and the following memcpy would run off the allocation. #2738 fixed this on main on 2026-07-23 but was not ported to the LTS branch, which still carries the unhardened pattern in all three files.

Contents

  1. 70f9fa28 cherry-picked unchanged. The three provisioning_client/src files are byte-identical to main at the allocation sites.
  2. One LTS-specific adaptation. The backported tests use IGNORED_ARG, which does not exist in the umock-c pinned by this branch (504193e6, March 2020); it was added later. Without this change the three provisioning UT suites fail to compile. All nine occurrences are pointer arguments, so they become IGNORED_PTR_ARG, matching the macro already used throughout these files on this branch.

Notes

  • No submodule bump needed: the c-utility pinned by lts_03_2025 (772a4f8b) already provides safe_add_size_t in safe_math.h, with a macro definition identical to the one on main.
  • No version bump here, matching how previous LTS backports on this branch were done (e.g. 5a48d3f); version/tagging happens at release time.
  • The cherry-pick also carries a small correctness fix in the HTTP path: payload_data is now set to NULL after the free(), so the malloc-failure branch no longer leaves a dangling pointer behind.
  • The duplicate http_proxy_io.h include removed from prov_transport_mqtt_common.c was genuinely duplicated on this branch too (lines 17 and 19); one include remains.
  • I was not able to compile or run the tests locally (no toolchain available in my environment), so I am relying on CI for build and UT validation. Please confirm the three provisioning UT suites pass before merging.

* Use safe_add_size_t when sizing DPS transport response buffers

The DPS HTTP, MQTT and AMQP transports sized their response buffers with
malloc(len + 1). Compute the allocation size with safe_add_size_t (as already
done elsewhere in the tree, e.g. uhttp) and fail the request if the addition
saturates, instead of allocating a mis-sized buffer.

Add a regression unit test per transport covering the saturating-length path.

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Address review feedback

- prov_transport_mqtt_common: drop duplicate http_proxy_io.h include
- prov_transport_http_client: correct payload-allocation error message
- amqp unit test: assert the error is surfaced to the register callback

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
(cherry picked from commit 70f9fa2)
The cherry-picked regression tests use IGNORED_ARG, which is only defined in
newer umock-c. lts_03_2025 pins umock-c 504193e6 (Mar 2020), where that macro
does not exist, so the three provisioning UT suites would fail to compile.

Replace IGNORED_ARG with IGNORED_PTR_ARG, matching the macro already used
throughout these test files on this branch. All nine occurrences are pointer
arguments, so IGNORED_PTR_ARG is the correct substitution.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@ewertons

Copy link
Copy Markdown
Contributor Author

CI on this PR is blocked by broken infrastructure on lts_03_2025, not by this change

Build 162517 failed 14 seconds in, during Initialize containers of the Setup stage:

Could not fetch access token for ACR. Please configure Managed Service Identity (MSI)
for Azure Container Registry with the appropriate permissions.

Nothing was compiled. Every Linux job on this branch still pulls containers from csdkcontainerregistry.azurecr.io via the csdk-containers endpoint and targets the sdk-c--* self-hosted pools; both were retired, and main moved off them in #2701. Setup gates every other stage, so any PR against this branch is red regardless of content.

I have opened #2747 to migrate this branch's pipeline to Microsoft-hosted agents, matching main. That PR validates itself, since Azure Pipelines builds the PR merge commit.

Suggested order: merge #2747 first, then rerun CI here. I will rebase this branch onto it once it lands.

Note this PR still has not been build- or test-validated anywhere; the three provisioning UT suites need to come back green before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant