This is an automated email from the ASF dual-hosted git repository.
mgrigorov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/avro.git
The following commit(s) were added to refs/heads/main by this push:
new 20772be73 [AVRO-3945] Add missing bounds check in the loop (#2812)
20772be73 is described below
commit 20772be733a3762d821aba57284cd462610238c1
Author: Mikhail Koviazin <[email protected]>
AuthorDate: Mon Mar 25 13:18:52 2024 +0200
[AVRO-3945] Add missing bounds check in the loop (#2812)
* [AVRO-3945] Add missing bounds checks for extra increments in the loop
This issue was found by cppcheck:
impl/json/JsonIO.cc:319:66: warning: Missing bounds check for extra
iterator increment in loop. [StlMissingComparison]
for (string::const_iterator it = s.begin(); it != s.end(); ++it) {
^
impl/json/JsonIO.cc:350:37: note: Missing bounds check for extra
iterator increment in loop.
char c = *++it;
^
impl/json/JsonIO.cc:319:66: note: Missing bounds check for extra
iterator increment in loop.
for (string::const_iterator it = s.begin(); it != s.end(); ++it) {
The original implementation contained a for-loop that incremented an
iterator on each iteration **and** if a backslash was found. This caused
a situtation when a malicious string could cause an invalid memory access,
because the iterator would reach **after** the `s.cend()` due to additional
increments in the loop body.
This commit fixes the issue.
* build.sh: sort unittests and add forgotten tests
---
lang/c++/build.sh | 13 ++++++++-----
lang/c++/impl/json/JsonIO.cc | 15 +++++++++++----
lang/c++/test/JsonTests.cc | 1 +
3 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/lang/c++/build.sh b/lang/c++/build.sh
index 81530ed0f..5eed350fa 100755
--- a/lang/c++/build.sh
+++ b/lang/c++/build.sh
@@ -86,14 +86,17 @@ case "$target" in
(cmake -S. -Bbuild -D CMAKE_BUILD_TYPE=Debug -D AVRO_ADD_PROTECTOR_FLAGS=1
&& cmake --build build \
&& ./build/buffertest \
&& ./build/unittest \
+ && ./build/AvrogencppTestReservedWords \
+ && ./build/AvrogencppTests \
&& ./build/CodecTests \
+ && ./build/CommonsSchemasTests \
&& ./build/CompilerTests \
- && ./build/StreamTests \
- && ./build/SpecificTests \
- && ./build/AvrogencppTests \
&& ./build/DataFileTests \
- && ./build/SchemaTests \
- && ./build/CommonsSchemasTests)
+ && ./build/JsonTests \
+ && ./build/LargeSchemaTests \
+ && ./build/SchemaTests \
+ && ./build/SpecificTests \
+ && ./build/StreamTests)
;;
xcode-test)
diff --git a/lang/c++/impl/json/JsonIO.cc b/lang/c++/impl/json/JsonIO.cc
index 62549484a..7f002ed99 100644
--- a/lang/c++/impl/json/JsonIO.cc
+++ b/lang/c++/impl/json/JsonIO.cc
@@ -316,10 +316,17 @@ JsonParser::Token JsonParser::tryString() {
string JsonParser::decodeString(const string &s, bool binary) {
string result;
- for (string::const_iterator it = s.begin(); it != s.end(); ++it) {
- char ch = *it;
+ const auto readNextByte = [](string::const_iterator &it, const
string::const_iterator &end) -> char {
+ if (it == end)
+ throw Exception("Unexpected EOF");
+ return *it++;
+ };
+ auto it = s.cbegin();
+ const auto end = s.cend();
+ while (it != end) {
+ char ch = *it++;
if (ch == '\\') {
- ch = *++it;
+ ch = readNextByte(it, end);
switch (ch) {
case '"':
case '\\':
@@ -347,7 +354,7 @@ string JsonParser::decodeString(const string &s, bool
binary) {
char e[4];
for (char &i : e) {
n *= 16;
- char c = *++it;
+ char c = readNextByte(it, end);
i = c;
if (isdigit(c)) {
n += c - '0';
diff --git a/lang/c++/test/JsonTests.cc b/lang/c++/test/JsonTests.cc
index da9722f30..10a100540 100644
--- a/lang/c++/test/JsonTests.cc
+++ b/lang/c++/test/JsonTests.cc
@@ -68,6 +68,7 @@ TestData<const char *> stringData[] = {
{R"("\/")", EntityType::String, "/", R"("\/")"},
{R"("\u20ac")", EntityType::String, "\xe2\x82\xac", R"("\u20ac")"},
{R"("\u03c0")", EntityType::String, "\xcf\x80", R"("\u03c0")"},
+ {R"("hello\n")", EntityType::String, "hello\n", R"("hello\n")"},
};
void testBool(const TestData<bool> &d) {
