Skip to content

Fix misaligned uint32 access in ring_pedersen / damgard_fujisaki serializers#55

Open
kozyilmaz wants to merge 1 commit into
fireblocks:mainfrom
kozyilmaz:serializer-alignment-fix
Open

Fix misaligned uint32 access in ring_pedersen / damgard_fujisaki serializers#55
kozyilmaz wants to merge 1 commit into
fireblocks:mainfrom
kozyilmaz:serializer-alignment-fix

Conversation

@kozyilmaz

Copy link
Copy Markdown
Contributor

Both read/wrote 4-byte length fields with (uint32_t)p after variable-length BN payloads, leaving the cursor unaligned, UB, SIGBUS on strict-alignment targets (damgard_fujisaki:691 is attacker-reachable via the public deserialize).

Add crypto/common/byte_io.h with host-endian load_u32/store_u32 (memcpy, no alignment requirement) and route every length-field access through it. Same wire bytes, no behavior change.

To not pollute mpc-lib codebase, all tests that prove the alignment issue on aarch64 are kept in a separate branch. You can find the test files in the https://github.com/kozyilmaz/mpc-lib/tree/serializer-alignment-tests branch (commits kozyilmaz@b87d761 and kozyilmaz@645a3ec)

You can checkout the branch and run the following commands to instrument and test

# Configure from scratch with UBSan (the misalignment is observed via UBSan)
cmake -S . -B build-ubsan \
  -DCMAKE_C_FLAGS="-fsanitize=undefined -fno-omit-frame-pointer -g -O1" \
  -DCMAKE_CXX_FLAGS="-fsanitize=undefined -fno-omit-frame-pointer -g -O1" \
  -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=undefined" \
  -DCMAKE_SHARED_LINKER_FLAGS="-fsanitize=undefined"

# Build the three alignment targets (pulls in cosigner + tests_main)
cmake --build build-ubsan --target ring_pedersen_alignment_test damgard_fujisaki_alignment_test align_repro -j"$(nproc)"

# Run — UBSan reports go to stderr; the tests pass (UBSan is the signal)
$ export UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=0

# Run tests:
./build-ubsan/test/crypto/ring_pedersen_alignment/ring_pedersen_alignment_test
./build-ubsan/test/crypto/damgard_fujisaki_alignment/damgard_fujisaki_alignment_test
./build-ubsan/test/cosigner/align_repro

Below are the test outputs on Ubuntu on aarch64:

$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 26.04 LTS
Release:	26.04
Codename:	resolute

$ uname -a
Linux utgard 7.0.0-22-generic #22-Ubuntu SMP PREEMPT_DYNAMIC Mon May 25 15:37:49 UTC 2026 aarch64 GNU/Linux

$ ./build-ubsan/test/crypto/ring_pedersen_alignment/ring_pedersen_alignment_test
/home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:278:9: runtime error: load of misaligned address 0xc21a3f87bac3 for type 'uint32_t', which requires 4 byte alignment
0xc21a3f87bac3: note: pointer points here
 a8  05 6b 1f ff 00 00 00 10  75 32 b1 92 98 67 99 dc  a5 16 ad ae db ab 24 61  9a c0 54 6a 05 c4 a4
              ^ 
    #0 0xc21a01bbfeb8 in ring_pedersen_public_deserialize_internal /home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:278
    #1 0xc21a01bc1154 in ring_pedersen_public_deserialize /home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:366
    #2 0xc21a01bbaf74 in exercise /home/loki/devel/mpc-lib/test/crypto/ring_pedersen_alignment/tests.cpp:56
    #3 0xc21a01bbc28c in C_A_T_C_H_T_E_S_T_0 /home/loki/devel/mpc-lib/test/crypto/ring_pedersen_alignment/tests.cpp:73
    #4 0xc21a01bc70f8 in Catch::TestInvokerAsFunction::invoke() const /home/loki/devel/mpc-lib/include/tests/catch.hpp:14334
    #5 0xc21a01bff170 in Catch::TestCase::invoke() const /home/loki/devel/mpc-lib/include/tests/catch.hpp:14173
    #6 0xc21a01bff348 in Catch::RunContext::invokeActiveTestCase() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13033
    #7 0xc21a01c65244 in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) /home/loki/devel/mpc-lib/include/tests/catch.hpp:13006
    #8 0xc21a01ca580c in Catch::RunContext::runTest(Catch::TestCase const&) /home/loki/devel/mpc-lib/include/tests/catch.hpp:12767
    #9 0xc21a01cb5240 in execute /home/loki/devel/mpc-lib/include/tests/catch.hpp:13360
    #10 0xc21a01cb5240 in Catch::Session::runInternal() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13566
    #11 0xc21a01cb6818 in Catch::Session::run() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13522
    #12 0xc21a01cb6fcc in int Catch::Session::run<char>(int, char const* const*) /home/loki/devel/mpc-lib/include/tests/catch.hpp:13244
    #13 0xc21a01cb6fcc in main /home/loki/devel/mpc-lib/include/tests/catch.hpp:17539
    #14 0xf433108e2f18 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:59
    #15 0xf433108e3058 in __libc_start_main_impl ../csu/libc-start.c:360
    #16 0xc21a01bba4ec in _start (/home/loki/devel/mpc-lib/build-ubsan/test/crypto/ring_pedersen_alignment/ring_pedersen_alignment_test+0x29a4ec) (BuildId: a5ae6bc15289e88e48746a779c3667c03e624c89)

/home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:290:9: runtime error: load of misaligned address 0xc21a3f87bbc6 for type 'uint32_t', which requires 4 byte alignment
0xc21a3f87bbc6: note: pointer points here
 4f 95 21 07 ff 00  00 00 10 75 aa 37 71 22  33 62 02 0a 5a 25 b4 f3  ca f7 3c 9e 74 0e 2b 26  e4 8c
             ^ 
    #0 0xc21a01bbff90 in ring_pedersen_public_deserialize_internal /home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:290
    #1 0xc21a01bc1154 in ring_pedersen_public_deserialize /home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:366
    #2 0xc21a01bbaf74 in exercise /home/loki/devel/mpc-lib/test/crypto/ring_pedersen_alignment/tests.cpp:56
    #3 0xc21a01bbc28c in C_A_T_C_H_T_E_S_T_0 /home/loki/devel/mpc-lib/test/crypto/ring_pedersen_alignment/tests.cpp:73
    #4 0xc21a01bc70f8 in Catch::TestInvokerAsFunction::invoke() const /home/loki/devel/mpc-lib/include/tests/catch.hpp:14334
    #5 0xc21a01bff170 in Catch::TestCase::invoke() const /home/loki/devel/mpc-lib/include/tests/catch.hpp:14173
    #6 0xc21a01bff348 in Catch::RunContext::invokeActiveTestCase() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13033
    #7 0xc21a01c65244 in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) /home/loki/devel/mpc-lib/include/tests/catch.hpp:13006
    #8 0xc21a01ca580c in Catch::RunContext::runTest(Catch::TestCase const&) /home/loki/devel/mpc-lib/include/tests/catch.hpp:12767
    #9 0xc21a01cb5240 in execute /home/loki/devel/mpc-lib/include/tests/catch.hpp:13360
    #10 0xc21a01cb5240 in Catch::Session::runInternal() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13566
    #11 0xc21a01cb6818 in Catch::Session::run() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13522
    #12 0xc21a01cb6fcc in int Catch::Session::run<char>(int, char const* const*) /home/loki/devel/mpc-lib/include/tests/catch.hpp:13244
    #13 0xc21a01cb6fcc in main /home/loki/devel/mpc-lib/include/tests/catch.hpp:17539
    #14 0xf433108e2f18 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:59
    #15 0xf433108e3058 in __libc_start_main_impl ../csu/libc-start.c:360
    #16 0xc21a01bba4ec in _start (/home/loki/devel/mpc-lib/build-ubsan/test/crypto/ring_pedersen_alignment/ring_pedersen_alignment_test+0x29a4ec) (BuildId: a5ae6bc15289e88e48746a779c3667c03e624c89)

/home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:244:19: runtime error: store to misaligned address 0xc21a3f8d4113 for type 'uint32_t', which requires 4 byte alignment
0xc21a3f8d4113: note: pointer points here
 a8  05 6b 1f 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00
              ^ 
    #0 0xc21a01bbfc70 in ring_pedersen_public_serialize_internal /home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:244
    #1 0xc21a01bc07c8 in ring_pedersen_public_serialize /home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:341
    #2 0xc21a01bbb498 in exercise /home/loki/devel/mpc-lib/test/crypto/ring_pedersen_alignment/tests.cpp:61
    #3 0xc21a01bbc28c in C_A_T_C_H_T_E_S_T_0 /home/loki/devel/mpc-lib/test/crypto/ring_pedersen_alignment/tests.cpp:73
    #4 0xc21a01bc70f8 in Catch::TestInvokerAsFunction::invoke() const /home/loki/devel/mpc-lib/include/tests/catch.hpp:14334
    #5 0xc21a01bff170 in Catch::TestCase::invoke() const /home/loki/devel/mpc-lib/include/tests/catch.hpp:14173
    #6 0xc21a01bff348 in Catch::RunContext::invokeActiveTestCase() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13033
    #7 0xc21a01c65244 in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) /home/loki/devel/mpc-lib/include/tests/catch.hpp:13006
    #8 0xc21a01ca580c in Catch::RunContext::runTest(Catch::TestCase const&) /home/loki/devel/mpc-lib/include/tests/catch.hpp:12767
    #9 0xc21a01cb5240 in execute /home/loki/devel/mpc-lib/include/tests/catch.hpp:13360
    #10 0xc21a01cb5240 in Catch::Session::runInternal() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13566
    #11 0xc21a01cb6818 in Catch::Session::run() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13522
    #12 0xc21a01cb6fcc in int Catch::Session::run<char>(int, char const* const*) /home/loki/devel/mpc-lib/include/tests/catch.hpp:13244
    #13 0xc21a01cb6fcc in main /home/loki/devel/mpc-lib/include/tests/catch.hpp:17539
    #14 0xf433108e2f18 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:59
    #15 0xf433108e3058 in __libc_start_main_impl ../csu/libc-start.c:360
    #16 0xc21a01bba4ec in _start (/home/loki/devel/mpc-lib/build-ubsan/test/crypto/ring_pedersen_alignment/ring_pedersen_alignment_test+0x29a4ec) (BuildId: a5ae6bc15289e88e48746a779c3667c03e624c89)

/home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:248:19: runtime error: store to misaligned address 0xc21a3f8d4216 for type 'uint32_t', which requires 4 byte alignment
0xc21a3f8d4216: note: pointer points here
 4f 95 21 07 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00
             ^ 
    #0 0xc21a01bbfcd0 in ring_pedersen_public_serialize_internal /home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:248
    #1 0xc21a01bc07c8 in ring_pedersen_public_serialize /home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:341
    #2 0xc21a01bbb498 in exercise /home/loki/devel/mpc-lib/test/crypto/ring_pedersen_alignment/tests.cpp:61
    #3 0xc21a01bbc28c in C_A_T_C_H_T_E_S_T_0 /home/loki/devel/mpc-lib/test/crypto/ring_pedersen_alignment/tests.cpp:73
    #4 0xc21a01bc70f8 in Catch::TestInvokerAsFunction::invoke() const /home/loki/devel/mpc-lib/include/tests/catch.hpp:14334
    #5 0xc21a01bff170 in Catch::TestCase::invoke() const /home/loki/devel/mpc-lib/include/tests/catch.hpp:14173
    #6 0xc21a01bff348 in Catch::RunContext::invokeActiveTestCase() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13033
    #7 0xc21a01c65244 in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) /home/loki/devel/mpc-lib/include/tests/catch.hpp:13006
    #8 0xc21a01ca580c in Catch::RunContext::runTest(Catch::TestCase const&) /home/loki/devel/mpc-lib/include/tests/catch.hpp:12767
    #9 0xc21a01cb5240 in execute /home/loki/devel/mpc-lib/include/tests/catch.hpp:13360
    #10 0xc21a01cb5240 in Catch::Session::runInternal() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13566
    #11 0xc21a01cb6818 in Catch::Session::run() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13522
    #12 0xc21a01cb6fcc in int Catch::Session::run<char>(int, char const* const*) /home/loki/devel/mpc-lib/include/tests/catch.hpp:13244
    #13 0xc21a01cb6fcc in main /home/loki/devel/mpc-lib/include/tests/catch.hpp:17539
    #14 0xf433108e2f18 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:59
    #15 0xf433108e3058 in __libc_start_main_impl ../csu/libc-start.c:360
    #16 0xc21a01bba4ec in _start (/home/loki/devel/mpc-lib/build-ubsan/test/crypto/ring_pedersen_alignment/ring_pedersen_alignment_test+0x29a4ec) (BuildId: a5ae6bc15289e88e48746a779c3667c03e624c89)

===============================================================================
All tests passed (12 assertions in 1 test case)


$ ./build-ubsan/test/crypto/damgard_fujisaki_alignment/damgard_fujisaki_alignment_test
/home/loki/devel/mpc-lib/src/common/crypto/commitments/damgard_fujisaki.c:691:22: runtime error: load of misaligned address 0xc6f66b5ddee3 for type 'const uint32_t', which requires 4 byte alignment
0xc6f66b5ddee3: note: pointer points here
 00  00 00 00 01 00 00 00 00  11 01 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00
              ^ 
    #0 0xfe4443dd4ebc in damgard_fujisaki_public_deserialize_internal /home/loki/devel/mpc-lib/src/common/crypto/commitments/damgard_fujisaki.c:691
    #1 0xfe4443dd82ac in damgard_fujisaki_public_deserialize /home/loki/devel/mpc-lib/src/common/crypto/commitments/damgard_fujisaki.c:743
    #2 0xc6f655da38c4 in C_A_T_C_H_T_E_S_T_0 /home/loki/devel/mpc-lib/test/crypto/damgard_fujisaki_alignment/tests.cpp:45
    #3 0xc6f655da7354 in Catch::TestInvokerAsFunction::invoke() const /home/loki/devel/mpc-lib/include/tests/catch.hpp:14334
    #4 0xc6f655ddf3cc in Catch::TestCase::invoke() const /home/loki/devel/mpc-lib/include/tests/catch.hpp:14173
    #5 0xc6f655ddf5a4 in Catch::RunContext::invokeActiveTestCase() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13033
    #6 0xc6f655e454a0 in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) /home/loki/devel/mpc-lib/include/tests/catch.hpp:13006
    #7 0xc6f655e85a68 in Catch::RunContext::runTest(Catch::TestCase const&) /home/loki/devel/mpc-lib/include/tests/catch.hpp:12767
    #8 0xc6f655e9549c in execute /home/loki/devel/mpc-lib/include/tests/catch.hpp:13360
    #9 0xc6f655e9549c in Catch::Session::runInternal() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13566
    #10 0xc6f655e96a74 in Catch::Session::run() /home/loki/devel/mpc-lib/include/tests/catch.hpp:13522
    #11 0xc6f655e97228 in int Catch::Session::run<char>(int, char const* const*) /home/loki/devel/mpc-lib/include/tests/catch.hpp:13244
    #12 0xc6f655e97228 in main /home/loki/devel/mpc-lib/include/tests/catch.hpp:17539
    #13 0xfe4442d92f18 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:59
    #14 0xfe4442d93058 in __libc_start_main_impl ../csu/libc-start.c:360
    #15 0xc6f655da33ec in _start (/home/loki/devel/mpc-lib/build-ubsan/test/crypto/damgard_fujisaki_alignment/damgard_fujisaki_alignment_test+0x2933ec) (BuildId: 33f0c39412d31ebdc0e27be79d4930fb732e4838)

===============================================================================
All tests passed (1 assertion in 1 test case)


$ ./build-ubsan/test/cosigner/align_repro
align_repro: 512 iterations of generate_setup_commitments(EDDSA_ED25519); under UBSan watch for a misaligned-uint32 report at ring_pedersen.c
created share for key key-0 n = 2
...
created share for key key-21 n = 2
/home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:248:19: runtime error: store to misaligned address 0xc85fa2c6e5a7 for type 'uint32_t', which requires 4 byte alignment
0xc85fa2c6e5a7: note: pointer points here
 f7 fc 0b 2e 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00
             ^ 
    #0 0xef0076c19be4 in ring_pedersen_public_serialize_internal /home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:248
    #1 0xef0076c1a6dc in ring_pedersen_public_serialize /home/loki/devel/mpc-lib/src/common/crypto/commitments/ring_pedersen.c:341
    #2 0xef0076daae84 in fireblocks::common::cosigner::cmp_setup_service::serialize_auxiliary_keys(fireblocks::common::cosigner::auxiliary_keys const&, std::vector<unsigned char, std::allocator<unsigned char> >&, std::vector<unsigned char, std::allocator<unsigned char> >&) /home/loki/devel/mpc-lib/src/common/cosigner/cmp_setup_service.cpp:615
    #3 0xef0076dac504 in fireblocks::common::cosigner::cmp_setup_service::create_setup_decommitment(elliptic_curve256_algebra_ctx const*, fireblocks::common::cosigner::auxiliary_keys const&, fireblocks::common::cosigner::setup_data const&, fireblocks::common::cosigner::setup_decommitment&) /home/loki/devel/mpc-lib/src/common/cosigner/cmp_setup_service.cpp:676
    #4 0xef0076db17b4 in fireblocks::common::cosigner::cmp_setup_service::generate_setup_commitments(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, cosigner_sign_algorithm, elliptic_curve256_algebra_ctx const*, std::vector<unsigned long, std::allocator<unsigned long> > const&, unsigned char, unsigned long, unsigned char const (&) [32], unsigned char const (*) [33], fireblocks::common::cosigner::commitment&) /home/loki/devel/mpc-lib/src/common/cosigner/cmp_setup_service.cpp:537
    #5 0xef0076dbf7c8 in fireblocks::common::cosigner::cmp_setup_service::generate_setup_commitments(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, cosigner_sign_algorithm, std::vector<unsigned long, std::allocator<unsigned long> > const&, unsigned char, unsigned long, fireblocks::common::cosigner::share_derivation_args const&, fireblocks::common::cosigner::commitment&) /home/loki/devel/mpc-lib/src/common/cosigner/cmp_setup_service.cpp:122
    #6 0xc85f8eee6a74 in main /home/loki/devel/mpc-lib/test/cosigner/align_repro.cpp:176
    #7 0xef0075bd2f18 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:59
    #8 0xef0075bd3058 in __libc_start_main_impl ../csu/libc-start.c:360
    #9 0xc85f8eed6aec in _start (/home/loki/devel/mpc-lib/build-ubsan/test/cosigner/align_repro+0x46aec) (BuildId: e2d3951f073642206c9af2c0db4d37c1127af696)

created share for key key-22 n = 2
...
align_repro: completed 512 iterations

All tests are fixed after this commit.

…alizers

Both read/wrote 4-byte length fields with *(uint32_t*)p after variable-length BN payloads, leaving the cursor unaligned, UB, SIGBUS on strict-alignment targets (damgard_fujisaki:691 is attacker-reachable via the public deserialize).

Add crypto/common/byte_io.h with host-endian load_u32/store_u32 (memcpy, no alignment requirement) and route every length-field access through it. Same wire bytes, no behavior change.

Signed-off-by: kozyilmaz <kazim@monolytic.com>
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