Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibCrypto: UnsignedBigInteger::export_data has unexpected alignment behavior #23578

Open
ADKaster opened this issue Mar 14, 2024 · 0 comments
Open
Labels
bug Something isn't working

Comments

@ADKaster
Copy link
Member

Given the signature:

size_t UnsignedBigInteger::export_data(Bytes data, bool remove_leading_zeros) const

I would expect that this method, when passed remove_leading_zeroes, will always left-align the first non-zero byte into the data buffer.

However, this is not how it works. It will always copy a multiple of Word (u32) bytes into the data buffer, and return the total number of 'meaningful' bytes copied.

This means that if data is larger than trimmed_length() * sizeof(Word) (or trimmed_byte_length()), it becomes quite annoying to recover only the meaningful bytes when passing remove_leading_zeroes=true.

In any case, the current behavior seems to be that if the BigInt has an actual byte length of 5, 6 or 7 bytes without leading zeroes (or other non-word-aligned size), the bytes of the first word are right-aligned to the first byte. This is super unintuitive. The parameter name is "remove_leading_zeroes", not "return_how_many_bytes_remain_after_leading_zeroes_removed_please_realign_your_buffer_pointer_accordingly".

This test should be changed like so

TEST_CASE(test_bigint_big_endian_export)
{
    auto number = "448378203247"_bigint;
    char exported[8] { 0 };
    auto exported_length = number.export_data({ exported, 8 }, true);
    EXPECT_EQ(exported_length, 5u);
-   EXPECT(memcmp(exported + 3, "hello", 5) == 0);
+   EXPECT(memcmp(exported, "hello", 5) == 0);
}

and still pass.

@ADKaster ADKaster added the bug Something isn't working label Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant