From 15e5129e06c8c17fc171125768ee60419801ef1e Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Sat, 18 Nov 2023 19:35:42 +0100 Subject: Avoid reading beyond end of stack dump buffer. --- .../searchlib/parsequery/stackdumpiterator.cpp | 26 ++++++++++++++++------ .../vespa/searchlib/parsequery/stackdumpiterator.h | 1 + vespalib/src/tests/compress/compress_test.cpp | 6 +++++ vespalib/src/vespa/vespalib/util/compress.h | 26 ++++++++++++++++++++++ 4 files changed, 52 insertions(+), 7 deletions(-) diff --git a/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp b/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp index d1a2c26a75d..b3939ab1a11 100644 --- a/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp +++ b/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp @@ -3,6 +3,7 @@ #include "stackdumpiterator.h" #include #include +#include using search::query::PredicateQueryTerm; @@ -57,8 +58,7 @@ SimpleQueryStackDumpIterator::~SimpleQueryStackDumpIterator() = default; vespalib::stringref SimpleQueryStackDumpIterator::read_stringref(const char *&p) { - uint64_t len; - p += vespalib::compress::Integer::decompressPositive(len, p); + uint64_t len = readCompressedPositiveInt(p); if ((p + len) > _bufEnd) throw false; vespalib::stringref result(p, len); p += len; @@ -68,9 +68,24 @@ SimpleQueryStackDumpIterator::read_stringref(const char *&p) uint64_t SimpleQueryStackDumpIterator::readCompressedPositiveInt(const char *&p) { + if (p > _bufEnd || !vespalib::compress::Integer::check_decompress_space(p, _bufEnd - p)) { + throw false; + } uint64_t tmp; p += vespalib::compress::Integer::decompressPositive(tmp, p); - if (p > _bufEnd) throw false; + assert(p <= _bufEnd); + return tmp; +} + +int64_t +SimpleQueryStackDumpIterator::readCompressedInt(const char *&p) +{ + if (p > _bufEnd || !vespalib::compress::Integer::check_decompress_positive_space(p, _bufEnd - p)) { + throw false; + } + int64_t tmp; + p += vespalib::compress::Integer::decompress(tmp, p); + assert(p <= _bufEnd); return tmp; } @@ -97,11 +112,8 @@ bool SimpleQueryStackDumpIterator::readNext() { _currType = ParseItem::GetType(typefield); if (ParseItem::GetFeature_Weight(typefield)) { - int64_t tmpLong; - if (p >= _bufEnd) return false; - p += vespalib::compress::Integer::decompress(tmpLong, p); + int64_t tmpLong = readCompressedInt(p); _currWeight.setPercent(tmpLong); - if (p > _bufEnd) return false; } else { _currWeight.setPercent(100); } diff --git a/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.h b/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.h index dece4ecc0b6..9f44e5f8912 100644 --- a/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.h +++ b/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.h @@ -49,6 +49,7 @@ private: VESPA_DLL_LOCAL vespalib::stringref read_stringref(const char *&p); VESPA_DLL_LOCAL uint64_t readCompressedPositiveInt(const char *&p); + VESPA_DLL_LOCAL int64_t readCompressedInt(const char *&p); VESPA_DLL_LOCAL bool readPredicate(const char *&p); VESPA_DLL_LOCAL bool readNN(const char *&p); VESPA_DLL_LOCAL bool readComplexTerm(const char *& p); diff --git a/vespalib/src/tests/compress/compress_test.cpp b/vespalib/src/tests/compress/compress_test.cpp index 04f71d5cc18..c4383a1c193 100644 --- a/vespalib/src/tests/compress/compress_test.cpp +++ b/vespalib/src/tests/compress/compress_test.cpp @@ -26,6 +26,9 @@ CompressTest::verifyPositiveNumber(uint64_t n, const uint8_t * expected, size_t for (size_t i(0); i < sz; i++) { EXPECT_EQUAL(expected[i], buf[i]); } + EXPECT_FALSE(Integer::check_decompress_positive_space(expected, 0u)); + EXPECT_FALSE(Integer::check_decompress_positive_space(expected, sz - 1)); + EXPECT_TRUE(Integer::check_decompress_positive_space(expected, sz)); uint64_t v(0); EXPECT_EQUAL(sz, Integer::decompressPositive(v, expected)); EXPECT_EQUAL(n, v); @@ -39,6 +42,9 @@ CompressTest::verifyNumber(int64_t n, const uint8_t * expected, size_t sz) { for (size_t i(0); i < sz; i++) { EXPECT_EQUAL(expected[i], buf[i]); } + EXPECT_FALSE(Integer::check_decompress_space(expected, 0u)); + EXPECT_FALSE(Integer::check_decompress_space(expected, sz - 1)); + EXPECT_TRUE(Integer::check_decompress_space(expected, sz)); int64_t v(0); EXPECT_EQUAL(sz, Integer::decompress(v, expected)); EXPECT_EQUAL(n, v); diff --git a/vespalib/src/vespa/vespalib/util/compress.h b/vespalib/src/vespa/vespalib/util/compress.h index 5cf2814f3ea..66234da0e32 100644 --- a/vespalib/src/vespa/vespalib/util/compress.h +++ b/vespalib/src/vespa/vespalib/util/compress.h @@ -105,6 +105,32 @@ public: } return numbytes; } + + static bool check_decompress_space(const void* srcv, size_t len) { + if (len == 0u) { + return false; + } + const uint8_t * src = static_cast(srcv); + const uint8_t c = src[0]; + if ((c & 0x40) != 0) { + return (((c & 0x20) != 0) ? (len >= 4u) : (len >= 2u)); + } else { + return true; + } + } + + static bool check_decompress_positive_space(const void* srcv, size_t len) { + if (len == 0u) { + return false; + } + const uint8_t * src = static_cast(srcv); + const uint8_t c = src[0]; + if ((c & 0x80) != 0) { + return (((c & 0x40) != 0) ? (len >= 4u) : (len >= 2u)); + } else { + return true; + } + } private: [[ noreturn ]] static void throw_too_big(int64_t n); [[ noreturn ]] static void throw_too_big(uint64_t n); -- cgit v1.2.3