Validation: fix out-of-bounds access when content ends in a string
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>
diff --git a/src/cborvalidation.c b/src/cborvalidation.c
index 08c3511..7075540 100644
--- a/src/cborvalidation.c
+++ b/src/cborvalidation.c
@@ -1,6 +1,6 @@
/****************************************************************************
**
-** Copyright (C) 2017 Intel Corporation
+** Copyright (C) 2019 Intel Corporation
**
** Permission is hereby granted, free of charge, to any person obtaining a copy
** of this software and associated documentation files (the "Software"), to deal
@@ -561,13 +561,17 @@
return err;
while (1) {
- err = validate_number(it, type, flags);
+ CborValue next;
+ err = _cbor_value_get_string_chunk(it, &ptr, &n, &next);
if (err)
return err;
+ if (ptr) {
+ err = validate_number(it, type, flags);
+ if (err)
+ return err;
+ }
- err = _cbor_value_get_string_chunk(it, &ptr, &n, it);
- if (err)
- return err;
+ *it = next;
if (!ptr)
break;