Skip to content

Fix 32-bit platform struct serialization issues#3

Open
seanjin99 wants to merge 1 commit intomainfrom
fix-32bit-struct-serialization
Open

Fix 32-bit platform struct serialization issues#3
seanjin99 wants to merge 1 commit intomainfrom
fix-32bit-struct-serialization

Conversation

@seanjin99
Copy link
Owner

  • 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.

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:

  1. Buffer overflow writing 16 bytes into 8-byte struct
  2. 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.
Verification
Platform Before After
macOS 64-bit (ENABLE_SVP=ON) 6678 PASSED 6678 PASSED
ARM32 embedded device 586 FAILED All PASSED

- 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
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