Verify dictionary property name length
This change verifies the dictionary property name length.
Tested:
Unit tested
Signed-off-by: Kasun Athukorala <kasunath@google.com>
Change-Id: I65f19d5bb1b8c9f838e427fd6d8ba17e2886bf9a
diff --git a/src/bej_dictionary.c b/src/bej_dictionary.c
index 15bbc89..e7dff3a 100644
--- a/src/bej_dictionary.c
+++ b/src/bej_dictionary.c
@@ -1,6 +1,7 @@
#include "bej_dictionary.h"
#include <stdbool.h>
+#include <stddef.h>
#include <stdio.h>
#include <string.h>
@@ -61,6 +62,37 @@
return true;
}
+static bool bejValidatePropertyNameLength(
+ const uint8_t* dictionary, uint32_t dictionarySize, uint16_t nameOffset,
+ uint8_t nameLength)
+{
+ if (nameLength == 0)
+ {
+ return true;
+ }
+ // After the property names, the dictionary has at least the
+ // uint8 CopyrightLength field. So we compare with dictionarySize - 1.
+ // nameOffset is uint16_t and nameLength is uint8_t so it cannot overflow
+ // uint32_t dictionarySize here.
+ if (((uint32_t)nameOffset) + (uint32_t)(nameLength) > (dictionarySize - 1))
+ {
+ fprintf(stderr, "Property name length goes beyond dictionary size\n");
+ return false;
+ }
+
+ // nameLength includes the NULL terminating. So the actual string size
+ // should be nameLength - 1.
+ const char* name = (const char*)dictionary + nameOffset;
+ const void* nullTerminator = memchr(name, '\0', nameLength);
+ if (nullTerminator == NULL ||
+ ((const char*)nullTerminator - name) != (ptrdiff_t)(nameLength - 1))
+ {
+ fprintf(stderr, "Property name length doesn't match actual length\n");
+ return false;
+ }
+ return true;
+}
+
uint16_t bejDictGetPropertyHeadOffset(void)
{
// First property is present soon after the dictionary header.
@@ -96,6 +128,13 @@
(const struct BejDictionaryProperty*)(dictionary + propertyOffset);
if (p->sequenceNumber == sequenceNumber)
{
+ if (!bejValidatePropertyNameLength(dictionary,
+ header->dictionarySize,
+ p->nameOffset, p->nameLength))
+ {
+ return bejErrorInvalidSize;
+ }
+
*property = p;
return 0;
}
diff --git a/test/bej_dictionary_test.cpp b/test/bej_dictionary_test.cpp
index 60b2d03..b54cab8 100644
--- a/test/bej_dictionary_test.cpp
+++ b/test/bej_dictionary_test.cpp
@@ -104,4 +104,31 @@
bejErrorInvalidPropertyOffset);
}
+TEST(BejDictionaryTest, InvalidPropertyNameLength)
+{
+ // Make a copy of the dummySimpleDict.
+ std::vector<uint8_t> modifiedDictionary = {dummySimpleDict.begin(),
+ dummySimpleDict.end()};
+
+ // Find a property and modify its nameLength to be out of bounds.
+ struct BejDictionaryHeader* header =
+ (struct BejDictionaryHeader*)modifiedDictionary.data();
+
+ ASSERT_GE(header->entryCount, 1);
+ struct BejDictionaryProperty* property =
+ (struct BejDictionaryProperty*)(modifiedDictionary.data() +
+ sizeof(BejDictionaryHeader));
+
+ // Increase the nameLength to go beyond the dictionary size.
+ property->nameLength = 255;
+
+ // Now try to get this property by sequence number; it should fail due to
+ // the invalid name length.
+ const struct BejDictionaryProperty* retrievedProperty = nullptr;
+ EXPECT_EQ(bejDictGetProperty(modifiedDictionary.data(),
+ bejDictGetPropertyHeadOffset(),
+ property->sequenceNumber, &retrievedProperty),
+ bejErrorInvalidSize);
+}
+
} // namespace libbej