Archived
Thiago Macieira
57b66a8353
We can only validate_number() if we know that we have a number to validate in the first place. If we've reached the end of our string, the content that follows is not necessarily a number (it could be a Break byte). More importantly, we could reach the end of the buffer. This issue was masked by the way we provided data to the parser. It always came from read-only memory becausee of QByteArray::fromRawData(), so valgrind never caught any issues. Using QByteArray directly wouldn't have helped because it always inserts a terminating null byte, which always validates as a correct number (unsigned 0) and fails to trigger valgrind. So we need to use malloc() directly to make Valgrind complain. And there was already a test that did: ==26543== Invalid read of size 1 ==26543== at 0x483EA10: memmove (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26543== by 0x43CEEA: read_bytes_unchecked (cborinternal_p.h:239) ==26543== by 0x43CFEC: extract_number_checked (cborinternal_p.h:286) ==26543== by 0x43D3E9: validate_number (cborvalidation.c:304) ==26543== by 0x43DC7B: validate_value (cborvalidation.c:551) ==26543== by 0x43DE8C: cbor_value_validate (cborvalidation.c:645) ==26543== by 0x4328D2: tst_Parser::strictValidation() (tst_parser.cpp:1637) ==26543== by 0x434632: tst_Parser::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (tst_parser.moc:291) ==26543== by 0x4C0B36D: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (qmetaobject.cpp:2310) ==26543== by 0x48673E9: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (qmetaobject.h:122) ==26543== by 0x4860256: QTest::TestMethods::invokeTestOnData(int) const (qtestcase.cpp:922) ==26543== by 0x4860D4B: QTest::TestMethods::invokeTest(int, char const*, QTest::WatchDog*) const (qtestcase.cpp:1121) ==26543== Address 0x61c4db1 is 0 bytes after a block of size 1 alloc'd ==26543== at 0x483777F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26543== by 0x403891: ParserWrapper::allocateMemory(unsigned long) (tst_parser.cpp:181) ==26543== by 0x436898: ParserWrapper::init(QByteArray const&) (tst_parser.cpp:126) ==26543== by 0x432712: tst_Parser::strictValidation() (tst_parser.cpp:1634) ==26543== by 0x434632: tst_Parser::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (tst_parser.moc:291) ==26543== by 0x4C0B36D: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (qmetaobject.cpp:2310) ==26543== by 0x48673E9: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (qmetaobject.h:122) ==26543== by 0x4860256: QTest::TestMethods::invokeTestOnData(int) const (qtestcase.cpp:922) ==26543== by 0x4860D4B: QTest::TestMethods::invokeTest(int, char const*, QTest::WatchDog*) const (qtestcase.cpp:1121) ==26543== by 0x4862083: QTest::TestMethods::invokeTests(QObject*) const (qtestcase.cpp:1465) ==26543== by 0x4862C14: QTest::qRun() (qtestcase.cpp:1903) ==26543== by 0x48626C3: QTest::qExec(QObject*, int, char**) (qtestcase.cpp:1792) ==26543== PASS : tst_Parser::strictValidation(bytearray-0) This commit goes further and makes it so an out-of-bounds access will cause a pagefault. Fixes #156. Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>