Avoid copying null in concat, unused + breaks views by paulocsanz · Pull Request #8198 · esp8266/Arduino (original) (raw)

Nice, so something like this would solve the problem?

void setup() { char ch = 'A'; String str; str.concat(&ch, 1); }

More like this...

diff --git a/tests/host/core/test_string.cpp b/tests/host/core/test_string.cpp
index 61931327..7105fa28 100644
--- a/tests/host/core/test_string.cpp
+++ b/tests/host/core/test_string.cpp
@@ -594,3 +594,12 @@ TEST_CASE("String chaining", "[core][String]")
     REQUIRE(static_cast<const void*>(result.c_str()) == static_cast<const void*>(ptr));
   }
 }
+
+TEST_CASE("String concat OOB #8198", "[core][String]")
+{
+    char *p = (char*)malloc(16);
+    memset(p, 'x', 16);
+    String s = "abcd";
+    s.concat(p, 16);
+    REQUIRE(!strcmp(s.c_str(), "abcdxxxxxxxxxxxxxxxx"));
+}

Which generates an error in valgrind on host tests

==2849== Invalid read of size 1
==2849==    at 0x4840270: memcpy@GLIBC_2.2.5 (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2849==    by 0x1EBA85: memmove (string_fortified.h:40)
==2849==    by 0x1EBA85: String::concat(char const*, unsigned int) (WString.cpp:308)
==2849==    by 0x1AEE10: ____C_A_T_C_H____T_E_S_T____598() (test_string.cpp:603)
==2849==    by 0x1D2CDE: Catch::RunContext::invokeActiveTestCase() [clone .isra.0] (catch.hpp:5473)
==2849==    by 0x1E0C47: Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (catch.hpp:5445)
==2849==    by 0x1E482D: Catch::RunContext::runTest(Catch::TestCase const&) (catch.hpp:5284)
==2849==    by 0x1E7EE8: Catch::Runner::runTests() (catch.hpp:5603)
==2849==    by 0x1E8206: Catch::Session::run() (catch.hpp:5734)
==2849==    by 0x1232F9: run (catch.hpp:5714)
==2849==    by 0x1232F9: main (catch.hpp:9278)
==2849==  Address 0x606b970 is 0 bytes after a block of size 16 alloc'd
==2849==    at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2849==    by 0x1AEDAA: ____C_A_T_C_H____T_E_S_T____598() (test_string.cpp:600)
==2849==    by 0x1D2CDE: Catch::RunContext::invokeActiveTestCase() [clone .isra.0] (catch.hpp:5473)
==2849==    by 0x1E0C47: Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (catch.hpp:5445)
==2849==    by 0x1E482D: Catch::RunContext::runTest(Catch::TestCase const&) (catch.hpp:5284)
==2849==    by 0x1E7EE8: Catch::Runner::runTests() (catch.hpp:5603)
==2849==    by 0x1E8206: Catch::Session::run() (catch.hpp:5734)
==2849==    by 0x1232F9: run (catch.hpp:5714)
==2849==    by 0x1232F9: main (catch.hpp:9278)