alexey-milovidov opened a new pull request, #536: URL: https://github.com/apache/datasketches-cpp/pull/536
`compact_theta_sketch_parser::parse` checks that the buffer holds 8 bytes, then reads header fields beyond those 8 bytes before checking the size again: | serial version | preamble longs | field read before the size check | |---|---|---| | 1 | 3 | `num_entries` (offset 8), `theta` (offset 16) | | 2 | 2 | `num_entries` (offset 8) | | 2 | 3 | `num_entries` (offset 8), `theta` (offset 16) | | 3 | 2 (not empty) | `num_entries` (offset 8) | So `compact_theta_sketch::deserialize(ptr, size)` and `wrapped_compact_theta_sketch::wrap(ptr, size)` read past the buffer when given a truncated sketch (a heap-buffer-overflow under AddressSanitizer). Also, the bytes read after the buffer decide the result. A serial version 1 or 2 sketch truncated to 8 bytes is accepted as an empty sketch if the memory after the buffer looks like an empty preamble. Otherwise it is rejected with an "at least N bytes expected" message where N depends on that memory. This change checks that the whole preamble is present before reading any field from it. The `std::istream` deserializer already does this implicitly, because it has to read the preamble before it can decide anything. Valid sketches are not affected: a serialized sketch always contains its full preamble, except the 8-byte empty sketches, which take a separate path. The new test case uses buffers that contain exactly the truncated bytes. Without the fix it fails under AddressSanitizer with a heap-buffer-overflow in `compact_theta_sketch_parser_impl.hpp`. Without a sanitizer, the serial version 1 and 2 cases were accepted as empty sketches. The ClickHouse AST fuzzer found this through `CAST(<string> AS AggregateFunction(uniqTheta, UInt64))`: https://github.com/ClickHouse/ClickHouse/issues/119595 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
