From 22f6fb4b63b96fcd9371d019fa323efcd1da1eef Mon Sep 17 00:00:00 2001 From: Czarek Nakamoto Date: Fri, 19 Apr 2024 16:37:42 +0200 Subject: [PATCH] fix regarding the issues raised during security audit In the polyseed-examples repository, the `utf8_nfc` and `utf8_nfkd` functions will never return a value exceeding `POLYSEED_STR_SIZE - 1` In your code, the utf8_norm function has variable return behavior that seems odd In case of a normalization error, the underlying normalizer will return a negative value, at which point your function just returns POLYSEED_STR_SIZE (this is unclear) In case the buffer isn't large enough, the normalizer will return the required buffer size but have undefined internal behavior, at which point your function returns a value exceeding POLYSEED_STR_SIZE Otherwise, it uses the normalizer's return value (indicating the written size) to continue with re-encoding tobtoht: Czarek Nakamoto: polyseed asserts that the return value < POLYSEED_STR_SIZE, so if normalization fails the program crashes.. > I think my idea was to have have polyseed check the return value and return an error code instead of asserting, which would in turn throw the "Unicode normalization failed" error > I'll upstream that. In the meantime you can replace the injected function with ```cpp inline size_t utf8_norm(const char* str, polyseed_str norm, utf8proc_option_t options) { utf8proc_int32_t buffer[POLYSEED_STR_SIZE]; utf8proc_ssize_t result; result = utf8proc_decompose(reinterpret_cast(str), 0, buffer, POLYSEED_STR_SIZE, options); if (result < 0 || result > (POLYSEED_STR_SIZE - 1)) { throw std::runtime_error("Unicode normalization failed"); } result = utf8proc_reencode(buffer, result, options); if (result < 0 || result > POLYSEED_STR_SIZE) { throw std::runtime_error("Unicode normalization failed"); } strcpy(norm, reinterpret_cast(buffer)); sodium_memzero(buffer, sizeof(buffer)); return result; } ``` --- patches/monero/0001-polyseed.patch | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/patches/monero/0001-polyseed.patch b/patches/monero/0001-polyseed.patch index 9e3c061..5a39c36 100644 --- a/patches/monero/0001-polyseed.patch +++ b/patches/monero/0001-polyseed.patch @@ -517,14 +517,14 @@ index 000000000..b26f37574 + utf8proc_ssize_t result; + + result = utf8proc_decompose(reinterpret_cast(str), 0, buffer, POLYSEED_STR_SIZE, options); -+ if (result < 0) { -+ return POLYSEED_STR_SIZE; ++ if (result < 0 || result > (POLYSEED_STR_SIZE - 1)) { ++ throw std::runtime_error("Unicode normalization failed"); + } -+ if (result > POLYSEED_STR_SIZE - 1) { -+ return result; -+ } -+ ++ + result = utf8proc_reencode(buffer, result, options); ++ if (result < 0 || result > POLYSEED_STR_SIZE) { ++ throw std::runtime_error("Unicode normalization failed"); ++ } + + strcpy(norm, reinterpret_cast(buffer)); + sodium_memzero(buffer, POLYSEED_STR_SIZE);