From 2b0fbb30c9f7da510271479bdf77a6764eb00b86 Mon Sep 17 00:00:00 2001
From: esaunders <esaunders@basistech.com>
Date: Tue, 20 Oct 2015 16:21:20 -0400
Subject: [PATCH] Added extra validation and handling of unknown registry
 types.

---
 rejistry++/src/ByteBuffer.cpp         | 17 ++++++++++++++++-
 rejistry++/src/ByteBuffer.h           |  5 +++++
 rejistry++/src/NKRecord.cpp           | 10 ++++++++++
 rejistry++/src/NKRecord.h             |  2 ++
 rejistry++/src/RegistryByteBuffer.cpp |  8 ++++++++
 rejistry++/src/VKRecord.cpp           | 10 ++++++++--
 rejistry++/src/VKRecord.h             |  2 ++
 7 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/rejistry++/src/ByteBuffer.cpp b/rejistry++/src/ByteBuffer.cpp
index 8d79f5269..de0ddf405 100644
--- a/rejistry++/src/ByteBuffer.cpp
+++ b/rejistry++/src/ByteBuffer.cpp
@@ -41,7 +41,9 @@ namespace Rejistry {
         }
 
         ByteBuffer::ByteBuffer(const ByteArray& buf, const uint32_t length) : Buffer(length) {
-            initializeBuffer(&buf[0], length);
+            if (buf.size() > 0) {
+                initializeBuffer(&buf[0], length);
+            }
         }
 
         void ByteBuffer::initializeBuffer(const uint8_t * buf, const uint32_t length) {
@@ -56,6 +58,11 @@ namespace Rejistry {
         }
 
         void ByteBuffer::get(ByteArray& dst, const uint32_t offset, const uint32_t length) {
+            if (length == 0) {
+                // No data requested.
+                return;
+            }
+
             if (offset > dst.size()) {
                 throw RegistryParseException("Offset is greater than destination buffer size.");
             }
@@ -64,6 +71,14 @@ namespace Rejistry {
                 throw RegistryParseException("Length is greater than available space in destination buffer.");
             }
 
+            if ((_position + offset) > _limit) {
+                throw RegistryParseException("Starting position is beyond end of buffer.");
+            }
+
+            if ((_position + offset + length) > _limit) {
+                throw RegistryParseException("Number of requested bytes exceeds buffer size.");
+            }
+
             memcpy(&dst[0], &_buffer[_position + offset], length);
             _position += offset;
         }
diff --git a/rejistry++/src/ByteBuffer.h b/rejistry++/src/ByteBuffer.h
index 1b656f793..fab387442 100644
--- a/rejistry++/src/ByteBuffer.h
+++ b/rejistry++/src/ByteBuffer.h
@@ -48,6 +48,7 @@ namespace Rejistry {
         ByteBuffer(const ByteArray& buf, const uint32_t length);
         virtual ~ByteBuffer() { _buffer.clear(); }
 
+        /// Get one byte from the current position in the buffer.
         uint8_t get(uint32_t offset) const;
 
         /**
@@ -61,8 +62,12 @@ namespace Rejistry {
          * @throws RegistryParseException
          */
         void get(ByteArray& dst, const uint32_t offset, const uint32_t length);
+
+        /// Get two bytes from the current position in the buffer.
         uint16_t getShort(uint32_t offset) const;
+        /// Get four bytes from the current position in the buffer.
         uint32_t getInt(uint32_t offset) const;
+        /// Get eight bytes from the current position in the buffer.
         uint64_t getLong(uint32_t offset) const;
 
     private:
diff --git a/rejistry++/src/NKRecord.cpp b/rejistry++/src/NKRecord.cpp
index 47670c235..b57685f9c 100644
--- a/rejistry++/src/NKRecord.cpp
+++ b/rejistry++/src/NKRecord.cpp
@@ -57,6 +57,11 @@ namespace Rejistry {
 
         int32_t offset = (int32_t)getDWord(CLASSNAME_OFFSET_OFFSET);
         uint16_t length = getWord(CLASSNAME_LENGTH_OFFSET);
+
+        if (length > MAX_NAME_LENGTH) {
+            throw RegistryParseException("Class name exceeds maximum length.");
+        }
+
         uint32_t classnameOffset = REGFHeader::FIRST_HBIN_OFFSET + offset;
         std::auto_ptr< Cell > c(new Cell(_buf, classnameOffset));
         if (c.get() == NULL) {
@@ -84,6 +89,11 @@ namespace Rejistry {
 
     std::wstring NKRecord::getName() const {
         uint32_t nameLength = getWord(NAME_LENGTH_OFFSET);
+
+        if (nameLength > MAX_NAME_LENGTH) {
+            throw RegistryParseException("Key name exceeds maximum length.");
+        }
+
         if (hasAsciiName()) {
             // TODO: This is a little hacky but it should work fine
             // for ASCII strings.
diff --git a/rejistry++/src/NKRecord.h b/rejistry++/src/NKRecord.h
index 2a2b73017..0c5a7ba64 100644
--- a/rejistry++/src/NKRecord.h
+++ b/rejistry++/src/NKRecord.h
@@ -163,6 +163,8 @@ namespace Rejistry {
         static const uint16_t CLASSNAME_LENGTH_OFFSET = 0x4A;
         static const uint16_t NAME_OFFSET = 0x4C;
 
+        static const uint8_t MAX_NAME_LENGTH = 255;
+
         NKRecord() {};
         NKRecord& operator=(const NKRecord &);
     };
diff --git a/rejistry++/src/RegistryByteBuffer.cpp b/rejistry++/src/RegistryByteBuffer.cpp
index 04202c0ab..165f8065a 100644
--- a/rejistry++/src/RegistryByteBuffer.cpp
+++ b/rejistry++/src/RegistryByteBuffer.cpp
@@ -66,6 +66,10 @@ namespace Rejistry {
     }
 
     std::string RegistryByteBuffer::getASCIIString(const uint32_t offset, const uint32_t length) const {
+        if (length == 0) {
+            return "";
+        }
+
         ByteBuffer::ByteArray &data = getData(offset, length);
 
         return std::string(data.begin(), data.end());
@@ -76,6 +80,10 @@ namespace Rejistry {
     }
 
     std::wstring RegistryByteBuffer::getUTF16String(const uint32_t offset, const uint32_t length) const {
+        if (length == 0) {
+            return L"";
+        }
+
         ByteBuffer::ByteArray &data = getData(offset, length);
         std::wstring_convert<std::codecvt_utf16<wchar_t, 0x10ffff, std::little_endian>, wchar_t> conv;
         std::wstring result = conv.from_bytes(reinterpret_cast<const char*>(&data[0]), reinterpret_cast<const char*>(&data[0] + length));
diff --git a/rejistry++/src/VKRecord.cpp b/rejistry++/src/VKRecord.cpp
index e222e7cce..cf00fb42a 100644
--- a/rejistry++/src/VKRecord.cpp
+++ b/rejistry++/src/VKRecord.cpp
@@ -59,6 +59,11 @@ namespace Rejistry {
         }
 
         uint32_t nameLength = getWord(NAME_LENGTH_OFFSET);
+
+        if (nameLength > MAX_NAME_LENGTH) {
+            throw RegistryParseException("Value name length exceeds maximum length.");
+        }
+
         if (hasAsciiName()) {
             // TODO: This is a little hacky but it should work fine
             // for ASCII strings.
@@ -75,7 +80,7 @@ namespace Rejistry {
 
     uint32_t VKRecord::getDataLength() const {
         uint32_t size = getDWord(DATA_LENGTH_OFFSET);
-        if (size > LARGE_DATA_SIZE){
+        if (size >= LARGE_DATA_SIZE){
             size -= LARGE_DATA_SIZE;
         }
         return size;
@@ -140,7 +145,8 @@ namespace Rejistry {
             data = new RegistryByteBuffer(new ByteBuffer(getData(DATA_OFFSET_OFFSET, 0x8), 0x8));
             break;
         default:
-            throw RegistryParseException("Unknown value type.");
+            // Unknown registry type. Create an empty buffer.
+            data = new RegistryByteBuffer(new ByteBuffer(0));
         }
 
         return new ValueData(data, getValueType());                                                            
diff --git a/rejistry++/src/VKRecord.h b/rejistry++/src/VKRecord.h
index c0a048fb0..5acc3fdda 100644
--- a/rejistry++/src/VKRecord.h
+++ b/rejistry++/src/VKRecord.h
@@ -149,6 +149,8 @@ namespace Rejistry {
         static const uint16_t DB_DATA_SIZE = 0x3FD8;
         static const uint32_t LARGE_DATA_SIZE = 0x80000000;
 
+        static const uint16_t MAX_NAME_LENGTH = 32767;
+
         VKRecord();
         VKRecord& operator=(const VKRecord &);
     };
-- 
GitLab