Open
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
- EC ElGamal: Convert sa_unwrap_parameters_ec_elgamal_s (uint64_t) to sa_unwrap_parameters_ec_elgamal (size_t) in ta.c - CENC: Fix size check to expect sa_subsample_length_s (16 bytes per entry) - CENC: Use converted subsample_length_s array in client serialization - Remove debug ERROR log with incorrect format specifier Fixes 586 test failures on 32-bit ARM platforms where size_t is 4 bytes but the wire protocol uses fixed 64-bit fields.
d4bd50e to
cb66f6b
Compare
riwoh
approved these changes
Feb 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem Summary
The SecAPI3 reference implementation uses a client-TA (Trusted Application) serialization protocol where data structures are passed between the client library and the TA. These structures use fixed 64-bit fields (uint64_t) for wire compatibility, but the internal TA code uses platform-native size_t fields. On 64-bit platforms, both are 8 bytes, so no conversion is needed. On 32-bit platforms, size_t is 4 bytes, causing struct size mismatches that corrupt data.
Root Cause Analysis
The SecAPI3 defines paired struct types:
Wire format (*_s suffix): Uses uint64_t fields for consistent size across platforms
Internal format: Uses size_t fields for native pointer arithmetic
sa_unwrap_parameters_ec_elgamal_ssa_unwrap_parameters_ec_elgamalsa_subsample_length_ssa_subsample_lengthBug 1: EC ElGamal Key Unwrap (ta.c:607-612)
Before (broken on 32-bit):
memcpy(¶meters_ec_elgamal, params[2].mem_ref, params[2].mem_ref_size);
algorithm_parameters = params[2].mem_ref;
The client sends sa_unwrap_parameters_ec_elgamal_s (16 bytes), but the code copies into sa_unwrap_parameters_ec_elgamal (8 bytes on 32-bit). This causes:
Buffer overflow writing 16 bytes into 8-byte struct
Misaligned field reads (offset reads key_length's upper 32 bits)
After fixed:
memcpy(¶meters_ec_elgamal_s, params[2].mem_ref, params[2].mem_ref_size);
parameters_ec_elgamal.offset = (size_t)parameters_ec_elgamal_s.offset;
parameters_ec_elgamal.key_length = (size_t)parameters_ec_elgamal_s.key_length;
algorithm_parameters = ¶meters_ec_elgamal;
Bug 2: CENC Size Validation (ta.c:1830)
Before (broken on 32-bit):
if (params[1].mem_ref_size != sizeof(sa_subsample_length) * sample.subsample_count)
The client sends sa_subsample_length_s array (16 bytes/entry), but validation expects sa_subsample_length (8 bytes/entry on 32-bit). Every CENC request fails validation.
After fixed:
if (params[1].mem_ref_size != sizeof(sa_subsample_length_s) * sample.subsample_count)
Bug 3: CENC Client Serialization (sa_process_common_encryption.c:120)
Before (broken on 32-bit):
Client sends original array (wrong struct type)
CREATE_PARAM(param1, samples[i].subsample_lengths, param1_size);
After fixed:
Client sends converted array with fixed-size fields
CREATE_PARAM(param1, subsample_length_s, param1_size);
Impact: Fixes 516 CENC "integer overflow" errors caused by TA reading misaligned data on 32 bits platform.