8000 Harden velocypack validator after fuzz testing (#15662) · arangodb/arangodb@6788542 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6788542

Browse files
jsteemannKVS85
jsteemann
and
KVS85
authored
Harden velocypack validator after fuzz testing (#15662)
* Harden velocypack validator after fuzz testing Changes are from arangodb/velocypack#111 * updated CHANGELOG * fix error found after fuzz testing * update to latest state * reallow custom types * reset nesting level on each parsing invocation Co-authored-by: Vadim Kondratyev <vadim@arangodb.com>
1 parent 5a8d281 commit 6788542

File tree

13 files changed

+213
-59
lines changed

13 files changed

+213
-59
lines changed

‎3rdParty/velocypack/include/velocypack/Builder.h

Lines changed: 3 additions & 0 deletions
10000
Original file line numberDiff line numberDiff line change
@@ -1099,6 +1099,9 @@ class Builder {
10991099
bool checkAttributeUniqueness(Slice obj) const;
11001100
bool checkAttributeUniquenessSorted(Slice obj) const;
11011101
bool checkAttributeUniquenessUnsorted(Slice obj) const;
1102+
1103+
ValueLength effectivePaddingForOneByteMembers() const noexcept;
1104+
ValueLength effectivePaddingForTwoByteMembers() const noexcept;
11021105
};
11031106

11041107
struct BuilderNonDeleter {

‎3rdParty/velocypack/include/velocypack/Exception.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ class Exception : public virtual std::exception {
5454
CannotTranslateKey = 21,
5555
KeyNotFound = 22, // not used anymore
5656
BadTupleSize = 23,
57+
TooDeepNesting = 24,
5758

5859
BuilderNotSealed = 30,
5960
BuilderNeedOpenObject = 31,

‎3rdParty/velocypack/include/velocypack/Options.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
#ifndef VELOCYPACK_OPTIONS_H
2626
#define VELOCYPACK_OPTIONS_H 1
2727

28+
#include <cstdint>
29+
#include <limits>
2830
#include <string>
2931

3032
#include "velocypack/velocypack-common.h"
@@ -102,7 +104,8 @@ struct Options {
102104
// clear builder before starting to parse in Parser
103105
bool clearBuilderBeforeParse = true;
104106

105-
// validate UTF-8 strings when JSON-parsing with Parser
107+
// validate UTF-8 strings when JSON-parsing with Parser or validating with
108+
// Validator
106109
bool validateUtf8Strings = false;
107110

108111
// validate that attribute names in Object values are actually
@@ -149,6 +152,10 @@ struct Options {
149152
// write tags to JSON output
150153
bool debugTags = false;
151154

155+
// max recursion level for object/array nesting. checked by Parser and
156+
// Validator.
157+
uint32_t nestingLimit = std::numeric_limits<uint32_t>::max();
158+
152159
// default options with the above settings
153160
static Options Defaults;
154161
};

‎3rdParty/velocypack/include/velocypack/Parser.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class Parser {
8080
uint8_t const* _start;
8181
std::size_t _size;
8282
std::size_t _pos;
83-
int _nesting;
83+
uint32_t _nesting;
8484

8585
public:
8686
Options const* options;
@@ -124,7 +124,7 @@ class Parser {
124124
_size(0),
125125
_pos(0),
126126
_nesting(0),
127-
options(options) {
127+
options(options) {
128128
if (VELOCYPACK_UNLIKELY(options == nullptr)) {
129129
throw Exception(Exception::InternalError, "Options cannot be a nullptr");
130130
}
@@ -184,6 +184,7 @@ class Parser {
184184
_start = start;
185185
_size = size;
186186
_pos = 0;
187+
_nesting = 0;
187188
if (options->clearBuilderBeforeParse) {
188189
_builder->clear();
189190
}
@@ -304,9 +305,9 @@ class Parser {
304305
return i;
305306
}
306307

307-
inline void increaseNesting() { ++_nesting; }
308+
void increaseNesting();
308309

309-
inline void decreaseNesting() { --_nesting; }
310+
void decreaseNesting() noexcept;
310311

311312
void parseNumber();
312313

‎3rdParty/velocypack/include/velocypack/Slice.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1242,8 +1242,13 @@ class Slice {
12421242
uint8_t tagsOffset(uint8_t const* start) const noexcept {
12431243
uint8_t ret = 0;
12441244

1245-
while(SliceStaticData::TypeMap[*start] == ValueType::Tagged) {
1245+
while (SliceStaticData::TypeMap[*start] == ValueType::Tagged) {
12461246
uint8_t offset = tagOffset(start);
1247+
VELOCYPACK_ASSERT(offset != 0);
1248+
if (VELOCYPACK_UNLIKELY(offset == 0)) {
1249+
// prevent endless loop
1250+
break;
1251+
}
12471252
ret += offset;
12481253
start += offset;
12491254
}
@@ -1317,6 +1322,9 @@ class Slice {
13171322

13181323
case ValueType::Tagged: {
13191324
uint8_t offset = tagsOffset(start);
1325+
if (VELOCYPACK_UNLIKELY(offset == 0)) {
1326+
throw Exception(Exception::InternalError, "Invalid tag data in byteSize()");
1327+
}
13201328
return byteSize(start + offset) + offset;
13211329
}
13221330

‎3rdParty/velocypack/include/velocypack/Validator.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
#include "velocypack/velocypack-common.h"
2929
#include "velocypack/Options.h"
3030

31+
#include <cstdint>
32+
3133
namespace arangodb {
3234
namespace velocypack {
3335
class Slice;
@@ -39,7 +41,6 @@ class Validator {
3941
explicit Validator(Options const* options = &Options::Defaults);
4042
~Validator() = default;
4143

42-
public:
4344
// validates a VelocyPack Slice value starting at ptr, with length bytes length
4445
// throws if the data is invalid
4546
bool validate(char const* ptr, std::size_t length, bool isSubPart = false) {
@@ -51,6 +52,9 @@ class Validator {
5152
bool validate(uint8_t const* ptr, std::size_t length, bool isSubPart = false);
5253

5354
private:
55+
void validatePart(uint8_t const* ptr, std::size_t length, bool isSubPart);
56+
57+
void validateTagged(uint8_t const* ptr, std::size_t length);
5458
void validateArray(uint8_t const* ptr, std::size_t length);
5559
void validateCompactArray(uint8_t const* ptr, std::size_t length);
5660
void validateUnindexedArray(uint8_t const* ptr, std::size_t length);
@@ -66,7 +70,7 @@ class Validator {
6670
Options const* options;
6771

6872
private:
69-
int _level;
73+
uint32_t _level;
7074
};
7175

7276
} // namespace arangodb::velocypack

‎3rdParty/velocypack/src/Builder.cpp

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,8 @@ Builder& Builder::close() {
681681
(head == 0x0b && (options->buildUnindexedObjects || n == 1))) {
682682
if (closeCompactArrayOrObject(pos, isArray, indexStart, indexEnd)) {
683683
// And, if desired, check attribute uniqueness:
684-
if (options->checkAttributeUniqueness &&
684+
if ((head == 0x0b || head == 0x14) &&
685+
options->checkAttributeUniqueness &&
685686
n > 1 &&
686687
!checkAttributeUniqueness(Slice(_start + pos))) {
687688
// duplicate attribute name!
@@ -706,7 +707,7 @@ Builder& Builder::close() {
706707
unsigned int offsetSize = 8;
707708
// can be 1, 2, 4 or 8 for the byte width of the offsets,
708709
// the byte length and the number of subvalues:
709-
if (_pos - pos + n - 6 <= 0xff) {
710+
if (_pos - pos + n - 6 + effectivePaddingForOneByteMembers() <= 0xff) {
710711
// We have so far used _pos - pos bytes, including the reserved 8
711712
// bytes for byte length and number of subvalues. In the 1-byte number
712713
// case we would win back 6 bytes but would need one byte per subvalue
@@ -715,7 +716,7 @@ Builder& Builder::close() {
715716
// One could move down things in the offsetSize == 2 case as well,
716717
// since we only need 4 bytes in the beginning. However, saving these
717718
// 4 bytes has been sacrificed on the Altar of Performance.
718-
} else if (_pos - pos + 2 * n <= 0xffff) {
719+
} else if (_pos - pos + 2 * n + effectivePaddingForTwoByteMembers() <= 0xffff) {
719720
offsetSize = 2;
720721
} else if (_pos - pos + 4 * n <= 0xffffffffu) {
721722
offsetSize = 4;
@@ -760,26 +761,61 @@ Builder& Builder::close() {
760761
_start[pos] += 2;
761762
} else { // offsetSize == 8
762763
_start[pos] += 3;
764+
// write number of items
763765
reserve(8);
764766
appendLengthUnchecked<8>(n);
765767
}
766768
}
767769

768770
// Fix the byte length in the beginning:
769-
ValueLength x = _pos - pos;
771+
ValueLength const byteLength = _pos - pos;
772+
ValueLength x = byteLength;
770773
for (unsigned int i = 1; i <= offsetSize; i++) {
771774
_start[pos + i] = x & 0xff;
772775
x >>= 8;
773776
}
774777

775778
if (offsetSize < 8) {
776-
x = n;
779+
ValueLength x = n;
777780
for (unsigned int i = offsetSize + 1; i <= 2 * offsetSize; i++) {
778781
_start[pos + i] = x & 0xff;
779782
x >>= 8;
780783
}
781784
}
782785

786+
#ifdef VELOCYPACK_DEBUG
787+
// make sure byte size and number of items are actually written correctly
788+
if (offsetSize == F438 1) {
789+
VELOCYPACK_ASSERT(_start[pos] == 0x0b);
790+
// read byteLength
791+
uint64_t v = readIntegerFixed<uint64_t, 1>(_start + pos + 1);
792+
VELOCYPACK_ASSERT(byteLength == v);
793+
// read byteLength n
794+
v = readIntegerFixed<uint64_t, 1>(_start + pos + 1 + offsetSize);
795+
VELOCYPACK_ASSERT(n == v);
796+
} else if (offsetSize == 2) {
797+
VELOCYPACK_ASSERT(_start[pos] == 0x0c);
798+
// read byteLength
799+
uint64_t v = readIntegerFixed<uint64_t, 2>(_start + pos + 1);
800+
VELOCYPACK_ASSERT(byteLength == v);
801+
// read byteLength n
802+
v = readIntegerFixed<uint64_t, 2>(_start + pos + 1 + offsetSize);
803+
VELOCYPACK_ASSERT(n == v);
804+
} else if (offsetSize == 4) {
805+
VELOCYPACK_ASSERT(_start[pos] == 0x0d);
806+
// read byteLength
807+
uint64_t v = readIntegerFixed<uint64_t, 4>(_start + pos + 1);
808+
VELOCYPACK_ASSERT(byteLength == v);
809+
// read byteLength n
810+
v = readIntegerFixed<uint64_t, 4>(_start + pos + 1 + offsetSize);
811+
VELOCYPACK_ASSERT(n == v);
812+
} else if (offsetSize == 8) {
813+
VELOCYPACK_ASSERT(_start[pos] == 0x0e);
814+
uint64_t v = readIntegerFixed<uint64_t, 4>(_start + pos + 1);
815+
VELOCYPACK_ASSERT(byteLength == v);
816+
}
817+
#endif
818+
783819
// And, if desired, check attribute uniqueness:
784820
if (options->checkAttributeUniqueness &&
785821
n > 1 &&
@@ -1309,4 +1345,14 @@ uint8_t* Builder::add(ArrayIterator&& sub) {
13091345
return _start + oldPos;
13101346
}
13111347

1348+
ValueLength Builder::effectivePaddingForOneByteMembers() const noexcept {
1349+
// 8 bytes - object length (1 byte) - number of items (1 byte) = 6 bytes
1350+
return (options->paddingBehavior == Options::PaddingBehavior::UsePadding ? 6 : 0);
1351+
}
1352+
1353+
ValueLength Builder::effectivePaddingForTwoByteMembers() const noexcept {
1354+
// 8 bytes - object length (2 bytes) - number of items (2 bytes) = 4 bytes
1355+
return (options->paddingBehavior == Options::PaddingBehavior::UsePadding ? 4 : 0);
1356+
}
1357+
13121358
static_assert(sizeof(double) == 8, "double is not 8 bytes");

‎3rdParty/velocypack/src/Exception.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ char const* Exception::message(ExceptionType type) noexcept {
6666
return "Key not found";
6767
case BadTupleSize:
6868
return "Array size does not match tuple size";
69+
case TooDeepNesting:
70+
return "Too deep nesting in Array/Object";
6971
case BuilderNotSealed:
7072
return "Builder value not yet sealed";
7173
case BuilderNeedOpenObject:

‎3rdParty/velocypack/src/Parser.cpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,17 @@ int Parser::skipWhiteSpace(char const* err) {
118118
throw Exception(Exception::ParseError, err);
119119
}
120120

121+
void Parser::increaseNesting() {
122+
if (++_nesting >= options->nestingLimit) {
123+
throw Exception(Exception::TooDeepNesting);
124+
}
125+
}
126+
127+
void Parser::decreaseNesting() noexcept {
128+
VELOCYPACK_ASSERT(_nesting > 0);
129+
--_nesting;
130+
}
131+
121132
// parses a number value
122133
void Parser::parseNumber() {
123134
std::size_t startPos = _pos;
@@ -418,16 +429,17 @@ void Parser::parseString() {
418429
void Parser::parseArray() {
419430
_builderPtr->addArray();
420431

432+
increaseNesting();
433+
421434
int i = skipWhiteSpace("Expecting item or ']'");
422435
if (i == ']') {
423436
// empty array
424437
++_pos; // the closing ']'
438+
decreaseNesting();
425439
_builderPtr->close();
426440
return;
427441
}
428442

429-
increaseNesting();
430-
431443
while (true) {
432444
// parse array element itself
433445
_builderPtr->reportAdd();
@@ -454,20 +466,20 @@ void Parser::parseArray() {
454466
void Parser::parseObject() {
455467
_builderPtr->addObject();
456468

469+
increaseNesting();
457470
int i = skipWhiteSpace("Expecting item or '}'");
458471
if (i == '}') {
459472
// empty object
460473
consume(); // the closing '}'. return value intentionally not checked
461474

462475
if (_nesting != 0 || !options->keepTopLevelOpen) {
463476
// only close if we've not been asked to keep top level open
477+
decreaseNesting();
464478
_builderPtr->close();
465479
}
466480
return;
467481
}
468482

469-
increaseNesting();
470-
471483
while (true) {
472484
// always expecting a string attribute name here
473485
if (VELOCYPACK_UNLIKELY(i != '"')) {

0 commit comments

Comments
 (0)
0