Skip to content

Comments

Fix 32bit struct serialization#163

Open
seanjin99 wants to merge 1 commit intordkcentral:mainfrom
seanjin99:fix-32bit-struct-serialization
Open

Fix 32bit struct serialization#163
seanjin99 wants to merge 1 commit intordkcentral:mainfrom
seanjin99:fix-32bit-struct-serialization

Conversation

@seanjin99
Copy link
Contributor

@seanjin99 seanjin99 commented Feb 17, 2026

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

Struct Wire Format (32-bit) Internal Format (32-bit) Mismatch
sa_unwrap_parameters_ec_elgamal_s 16 bytes (2×uint64_t) N/A N/A
sa_unwrap_parameters_ec_elgamal N/A 8 bytes (2×size_t) 8 bytes
sa_subsample_length_s 16 bytes (2×uint64_t) N/A N/A
sa_subsample_length N/A 8 bytes (2×size_t) 8 bytes

Bug 1: EC ElGamal Key Unwrap (ta.c:607-612)
Before (broken on 32-bit):
memcpy(&parameters_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(&parameters_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 = &parameters_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.

Platform Before After
macOS 64-bit (ENABLE_SVP=ON) 6678 PASSED 6678 PASSED
ARM32 embedded device 586 FAILED All PASSED

@github-actions
Copy link

github-actions bot commented Feb 17, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

- 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.
@seanjin99 seanjin99 force-pushed the fix-32bit-struct-serialization branch from d4bd50e to cb66f6b Compare February 17, 2026 16:39
@riwoh riwoh requested a review from mhabrat February 18, 2026 15:43
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.

2 participants