diff options
| author | Czarek Nakamoto <cyjan@mrcyjanek.net> | 2024-04-19 16:37:42 +0200 |
|---|---|---|
| committer | Czarek Nakamoto <cyjan@mrcyjanek.net> | 2024-04-19 16:37:42 +0200 |
| commit | 22f6fb4b63b96fcd9371d019fa323efcd1da1eef (patch) | |
| tree | 39c6ae4baf8bf21cba523dc10de7494782ea4ffd | |
| parent | 05569f7b80560271071fc2bea46836b40a3f8277 (diff) | |
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<const uint8_t*>(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<const char*>(buffer));
sodium_memzero(buffer, sizeof(buffer));
return result;
}
```
| -rw-r--r-- | patches/monero/0001-polyseed.patch | 12 |
1 files 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<const uint8_t*>(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<const char*>(buffer)); + sodium_memzero(buffer, POLYSEED_STR_SIZE); |
