gh-Mbed-TLS/TF-PSA-Crypto

Watch

Fork 0

Defensive bounds check in psa_wipe_tag_output_buffer() #799

New issue

Open

opened 2026-06-02 01:30:09 -05:00 by Tanish-26 · 1 comment

Tanish-26 commented

2026-06-02 01:30:09 -05:00

(Migrated from github.com)

Summary

psa_wipe_tag_output_buffer() relies on a documented caller-enforced invariant (output_buffer_length <= output_buffer_size) but does not validate it internally. Adding a defensive assertion or bounds check would improve robustness and help detect future caller bugs.

System information

TF-PSA-Crypto version (number or commit id):
Current development branch (please replace with exact commit ID)

Operating system and version:
macOS (please replace with exact version)

Configuration (if not default, please attach crypto_config.h):
Default configuration

Compiler and options (if you used a pre-built binary, please indicate how you obtained it):
Clang/GCC default settings

Additional environment information:
None

Expected behavior

The function should either:

Assert that output_buffer_length <= output_buffer_size, or
Defensively clamp the value before using it in pointer arithmetic and size calculations.

This would make invariant violations easier to diagnose and prevent unexpected behavior if future code changes introduce an invalid caller.

Actual behavior

The function currently assumes the invariant holds:

if (status == PSA_SUCCESS) {
    offset = output_buffer_length;
}

memset(output_buffer + offset, '!',
       output_buffer_size - offset);

If a caller were to pass output_buffer_length > output_buffer_size, the subtraction would underflow and the resulting memset() call would operate on an unintended size.

The function documentation already states that output_buffer_length must be less than output_buffer_size, so the current implementation relies entirely on callers respecting that contract.

Steps to reproduce

Inspect core/psa_crypto.c.
Locate psa_wipe_tag_output_buffer().
Observe that output_buffer_length is copied into offset.
Observe that no validation is performed before:

memset(output_buffer + offset,
       '!',
       output_buffer_size - offset);

Consider a hypothetical invariant violation where:

output_buffer_length > output_buffer_size

which would cause the size calculation to underflow.

Additional information

I have not identified a reachable exploit path through the public PSA APIs and am not reporting this as a security vulnerability.

This issue is intended as a defense-in-depth / hardening suggestion. A possible improvement would be:

MBEDTLS_PSA_CRYPTO_ASSERT(offset <= output_buffer_size);

or:

if (offset > output_buffer_size) {
    offset = output_buffer_size;
}

before the final memset() call.

### Summary `psa_wipe_tag_output_buffer()` relies on a documented caller-enforced invariant (`output_buffer_length <= output_buffer_size`) but does not validate it internally. Adding a defensive assertion or bounds check would improve robustness and help detect future caller bugs. ### System information TF-PSA-Crypto version (number or commit id): Current development branch (please replace with exact commit ID) Operating system and version: macOS (please replace with exact version) Configuration (if not default, please attach `crypto_config.h`): Default configuration Compiler and options (if you used a pre-built binary, please indicate how you obtained it): Clang/GCC default settings Additional environment information: None ### Expected behavior The function should either: * Assert that `output_buffer_length <= output_buffer_size`, or * Defensively clamp the value before using it in pointer arithmetic and size calculations. This would make invariant violations easier to diagnose and prevent unexpected behavior if future code changes introduce an invalid caller. ### Actual behavior The function currently assumes the invariant holds: ```c if (status == PSA_SUCCESS) { offset = output_buffer_length; } memset(output_buffer + offset, '!', output_buffer_size - offset); ``` If a caller were to pass `output_buffer_length > output_buffer_size`, the subtraction would underflow and the resulting `memset()` call would operate on an unintended size. The function documentation already states that `output_buffer_length` must be less than `output_buffer_size`, so the current implementation relies entirely on callers respecting that contract. ### Steps to reproduce 1. Inspect `core/psa_crypto.c`. 2. Locate `psa_wipe_tag_output_buffer()`. 3. Observe that `output_buffer_length` is copied into `offset`. 4. Observe that no validation is performed before: ```c memset(output_buffer + offset, '!', output_buffer_size - offset); ``` 5. Consider a hypothetical invariant violation where: ```c output_buffer_length > output_buffer_size ``` which would cause the size calculation to underflow. ### Additional information I have not identified a reachable exploit path through the public PSA APIs and am not reporting this as a security vulnerability. This issue is intended as a defense-in-depth / hardening suggestion. A possible improvement would be: ```c MBEDTLS_PSA_CRYPTO_ASSERT(offset <= output_buffer_size); ``` or: ```c if (offset > output_buffer_size) { offset = output_buffer_size; } ``` before the final `memset()` call.

elxecutor commented

2026-07-31 15:49:25 -05:00

(Migrated from github.com)

I'd like to take this one.

👍 2

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Mbed-TLS/TF-PSA-Crypto#799

No description provided.

Rows
Columns