Skip to content

Issues discovered through fuzzing #166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
blaugold wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Issues discovered through fuzzing #166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5db1507
Fix count of mutable copy of dict with undefined value
blaugold Sep 28, 2022
230896f
Fix assert to handle `nan`
blaugold Sep 28, 2022
ce6afd2
Expand validation of untrusted Fleece data
blaugold Sep 28, 2022
5729d30
Handle binary and int values in `ValueSlot::copyValue`
blaugold Sep 28, 2022
a371d4b
Fix comparing `nullslice` and empty slice
blaugold Sep 30, 2022
da31ee9
Fix count of dicts with parent
blaugold Sep 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions Fleece/API_Impl/FLSlice.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,28 @@ FL_ASSUME_NONNULL_BEGIN

__hot
bool FLSlice_Equal(FLSlice a, FLSlice b) noexcept {
return a.size == b.size && FLMemCmp(a.buf, b.buf, a.size) == 0;
if (a.size == b.size) {
if (a.size == 0)
// Check wether both slices are the nullslice or empty slices.
return (a.buf == nullptr) == (b.buf == nullptr);
else
return FLMemCmp(a.buf, b.buf, a.size) == 0;
}
return false;
}


__hot
int FLSlice_Compare(FLSlice a, FLSlice b) noexcept {
// Optimized for speed, not simplicity!
if (a.size == b.size)
return FLMemCmp(a.buf, b.buf, a.size);
if (a.size == 0) {
// Sort the nullsice before an empty slice.
if ((a.buf == nullptr) == (b.buf == nullptr))
return 0;
else
return a.buf == nullptr ? -1 : 1;
} else
return FLMemCmp(a.buf, b.buf, a.size);
else if (a.size < b.size) {
int result = FLMemCmp(a.buf, b.buf, a.size);
return result ? result : -1;
Expand Down
52 changes: 26 additions & 26 deletions Fleece/Core/Dict.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,25 +65,25 @@ namespace fleece { namespace impl {

template <class KEY>
__hot
const Value* finishGet(const Value *keyFound, KEY &keyToFind) const noexcept {
const Value* finishGet(const Value *keyFound, KEY &keyToFind, bool returnUndefined) const noexcept {
if (keyFound) {
auto value = deref(next(keyFound));
if (_usuallyFalse(value->isUndefined()))
if (!returnUndefined && _usuallyFalse(value->isUndefined()))
value = nullptr;
return value;
} else {
const Dict *parent = getParent();
return parent ? parent->get(keyToFind) : nullptr;
return parent ? parent->get(keyToFind, returnUndefined) : nullptr;
}
}

__hot
inline const Value* getUnshared(slice keyToFind) const noexcept {
inline const Value* getUnshared(slice keyToFind, bool returnUndefined) const noexcept {
auto key = search(keyToFind, [](slice target, const Value *val) {
countComparison();
return compareKeys(target, val);
});
return finishGet(key, keyToFind);
return finishGet(key, keyToFind, returnUndefined);
}

__hot
Expand All @@ -95,25 +95,25 @@ namespace fleece { namespace impl {
}

__hot
inline const Value* get(int keyToFind) const noexcept {
inline const Value* get(int keyToFind, bool returnUndefined= false) const noexcept {
assert_precondition(keyToFind >= 0);
return finishGet(search(keyToFind), keyToFind);
return finishGet(search(keyToFind), keyToFind, returnUndefined);
}

__hot
inline const Value* get(slice keyToFind, SharedKeys *sharedKeys =nullptr) const noexcept {
inline const Value* get(slice keyToFind, SharedKeys *sharedKeys =nullptr, bool returnUndefined= false) const noexcept {
if (!sharedKeys && usesSharedKeys()) {
sharedKeys = findSharedKeys();
assert_precondition(sharedKeys || gDisableNecessarySharedKeysCheck);
}
int encoded;
if (sharedKeys && lookupSharedKey(keyToFind, sharedKeys, encoded))
return get(encoded);
return getUnshared(keyToFind);
return get(encoded, returnUndefined);
return getUnshared(keyToFind, returnUndefined);
}

__hot
const Value* get(Dict::key &keyToFind) const noexcept {
const Value* get(Dict::key &keyToFind, bool returnUndefined= false) const noexcept {
auto sharedKeys = keyToFind._sharedKeys;
if (!sharedKeys && usesSharedKeys()) {
sharedKeys = findSharedKeys();
Expand All @@ -123,21 +123,21 @@ namespace fleece { namespace impl {
if (_usuallyTrue(sharedKeys != nullptr)) {
// Look for a numeric key first:
if (_usuallyTrue(keyToFind._hasNumericKey))
return get(keyToFind._numericKey);
return get(keyToFind._numericKey, returnUndefined);
// Key was not registered last we checked; see if dict contains any new keys:
if (_usuallyFalse(_count == 0))
return nullptr;
if (lookupSharedKey(keyToFind._rawString, sharedKeys, keyToFind._numericKey)) {
keyToFind._hasNumericKey = true;
return get(keyToFind._numericKey);
return get(keyToFind._numericKey, returnUndefined);
}
}

// Look up by string:
const Value *key = findKeyByHint(keyToFind);
if (!key)
key = findKeyBySearch(keyToFind);
return finishGet(key, keyToFind);
return finishGet(key, keyToFind, returnUndefined);
}

key_t encodeKey(slice keyString, SharedKeys *sharedKeys) const noexcept {
Expand Down Expand Up @@ -278,7 +278,7 @@ namespace fleece { namespace impl {


__hot
static int compareKeys(const Value *keyToFind, const Value *key, bool wide) {
int compareKeys(const Value *keyToFind, const Value *key, bool wide) {
if (wide)
return dictImpl<true>::compareKeys(keyToFind, key);
else
Expand Down Expand Up @@ -309,7 +309,7 @@ namespace fleece { namespace impl {
if (_usuallyFalse(isMutable()))
return heapDict()->count();
Array::impl imp(this);
if (_usuallyFalse(imp._count > 1 && isMagicParentKey(imp._first))) {
if (_usuallyFalse(imp._count >= 1 && isMagicParentKey(imp._first))) {
// Dict has a parent; this makes counting much more expensive!
uint32_t c = 0;
for (iterator i(this); i; ++i)
Expand All @@ -327,26 +327,26 @@ namespace fleece { namespace impl {
}

__hot
const Value* Dict::get(slice keyToFind) const noexcept {
const Value* Dict::get(slice keyToFind, bool returnUndefined) const noexcept {
if (_usuallyFalse(isMutable()))
return heapDict()->get(keyToFind);
if (isWideArray())
return dictImpl<true>(this).get(keyToFind);
return dictImpl<true>(this).get(keyToFind, nullptr, returnUndefined);
else
return dictImpl<false>(this).get(keyToFind);
return dictImpl<false>(this).get(keyToFind, nullptr, returnUndefined);
}

__hot
const Value* Dict::get(int keyToFind) const noexcept {
const Value* Dict::get(int keyToFind, bool returnUndefined) const noexcept {
if (_usuallyFalse(isMutable()))
return heapDict()->get(keyToFind);
else if (isWideArray())
return dictImpl<true>(this).get(keyToFind);
return dictImpl<true>(this).get(keyToFind, returnUndefined);
else
return dictImpl<false>(this).get(keyToFind);
return dictImpl<false>(this).get(keyToFind, returnUndefined);
}

const Value* Dict::get(key &keyToFind) const noexcept {
const Value* Dict::get(key &keyToFind, bool returnUndefined) const noexcept {
if (_usuallyFalse(isMutable()))
return heapDict()->get(keyToFind);
else if (isWideArray())
Expand All @@ -356,13 +356,13 @@ namespace fleece { namespace impl {
}

__hot
const Value* Dict::get(const key_t &keyToFind) const noexcept {
const Value* Dict::get(const key_t &keyToFind, bool returnUndefined) const noexcept {
if (_usuallyFalse(isMutable()))
return heapDict()->get(keyToFind);
else if (keyToFind.shared())
return get(keyToFind.asInt());
return get(keyToFind.asInt(), returnUndefined);
else
return get(keyToFind.asString());
return get(keyToFind.asString(), returnUndefined);
}

key_t Dict::encodeKey(slice keyString, SharedKeys *sharedKeys) const noexcept {
Expand Down
13 changes: 8 additions & 5 deletions Fleece/Core/Dict.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ namespace fleece { namespace impl {
class SharedKeys;
class key_t;

/** Compares Dict keys in the order in which they must appear in Fleece data. */
int compareKeys(const Value *a, const Value *b, bool wide);

/** A Value that's a dictionary/map */
class Dict : public Value {
public:
Expand All @@ -30,10 +33,10 @@ namespace fleece { namespace impl {
bool empty() const noexcept FLPURE;

/** Looks up the Value for a string key. */
const Value* get(slice keyToFind) const noexcept FLPURE;
const Value* get(slice keyToFind, bool returnUndefined= false) const noexcept FLPURE;

/** Looks up the Value for an integer (shared) key. */
const Value* get(int numericKeyToFind) const noexcept FLPURE;
const Value* get(int numericKeyToFind, bool returnUndefined= false) const noexcept FLPURE;

/** If this array is mutable, returns the equivalent MutableArray*, else returns nullptr. */
MutableDict* asMutable() const noexcept FLPURE;
Expand Down Expand Up @@ -72,7 +75,7 @@ namespace fleece { namespace impl {
key(const key&) =delete;
private:
void setSharedKeys(SharedKeys*);

slice const _rawString;
SharedKeys* _sharedKeys {nullptr};
uint32_t _hint {0xFFFFFFFF};
Expand All @@ -84,9 +87,9 @@ namespace fleece { namespace impl {

/** Looks up the Value for a key, in a form that can cache the key's Fleece object.
Using the Fleece object is significantly faster than a normal get. */
const Value* get(key&) const noexcept;
const Value* get(key&, bool returnUndefined= false) const noexcept;

const Value* get(const key_t&) const noexcept;
const Value* get(const key_t&, bool returnUndefined= false) const noexcept;

constexpr Dict() :Value(internal::kDictTag, 0, 0) { }

Expand Down
5 changes: 3 additions & 2 deletions Fleece/Core/Pointer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,11 @@ namespace fleece { namespace impl { namespace internal {
}


bool Pointer::validate(bool wide, const void *dataStart) const noexcept{
bool Pointer::validate(bool wide, const void *dataStart,
bool checkSharedKeyExists) const noexcept{
const void *dataEnd = this;
const Value *target = carefulDeref(wide, dataStart, dataEnd);
return target && target->validate(dataStart, dataEnd);
return target && target->validate(dataStart, dataEnd, checkSharedKeyExists);
}


Expand Down
3 changes: 2 additions & 1 deletion Fleece/Core/Pointer.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ namespace fleece { namespace impl { namespace internal {
const void* &dataEnd) const noexcept;


bool validate(bool wide, const void *dataStart) const noexcept FLPURE;
bool validate(bool wide, const void *dataStart,
bool checkSharedKeyExists) const noexcept FLPURE;

private:
// Byte offset as interpreted prior to the 'extern' flag
Expand Down
5 changes: 4 additions & 1 deletion Fleece/Core/SharedKeys.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ namespace fleece { namespace impl {
alloc_slice stateData() const;
void writeState(Encoder &enc) const;

/** Sets the maximum length of string that can be mapped. (Defaults to 16 bytes.) */
/** The maximum length of string that can be mapped. */
size_t maxKeyLength() {return _maxKeyLength;}

/** Sets maxKeyLength. (Defaults to 16 bytes.) */
void setMaxKeyLength(size_t m) {_maxKeyLength = m;}

/** The number of stored keys. */
Expand Down
2 changes: 1 addition & 1 deletion Fleece/Core/Value+Dump.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ namespace fleece { namespace impl {


bool Value::dump(slice data, std::ostream &out) {
auto root = fromData(data);
auto root = fromData(data, /*checkSharedKeyExists=*/ false);
if (!root)
return false;
// Walk the tree and collect every value with its address:
Expand Down
Loading