better handle corrupt packets that would cause unreasonably deep recursion

libraries/octree/src/Octree.cpp

@@ -231,8 +231,18 @@ OctreeElement* Octree::nodeForOctalCode(OctreeElement* ancestorElement,
 }
 
 // returns the element created!
-OctreeElement* Octree::createMissingElement(OctreeElement* lastParentElement, const unsigned char* codeToReach) {
+OctreeElement* Octree::createMissingElement(OctreeElement* lastParentElement, const unsigned char* codeToReach, int recursionCount) {
+
+    if (recursionCount > DANGEROUSLY_DEEP_RECURSION) {
+        static QString repeatedMessage
+            = LogHandler::getInstance().addRepeatedMessageRegex(
+                    "Octree::createMissingElement\\(\\) reached DANGEROUSLY_DEEP_RECURSION, bailing!");
+
+        qDebug() << "Octree::createMissingElement() reached DANGEROUSLY_DEEP_RECURSION, bailing!";
+        return lastParentElement;
+    }
     int indexOfNewChild = branchIndexWithDescendant(lastParentElement->getOctalCode(), codeToReach);
+
     // If this parent element is a leaf, then you know the child path doesn't exist, so deal with
     // breaking up the leaf first, which will also create a child path
     if (lastParentElement->requiresSplit()) {
@@ -246,7 +256,7 @@ OctreeElement* Octree::createMissingElement(OctreeElement* lastParentElement, co
     if (*lastParentElement->getChildAtIndex(indexOfNewChild)->getOctalCode() == *codeToReach) {
         return lastParentElement->getChildAtIndex(indexOfNewChild);
     } else {
-        return createMissingElement(lastParentElement->getChildAtIndex(indexOfNewChild), codeToReach);
+        return createMissingElement(lastParentElement->getChildAtIndex(indexOfNewChild), codeToReach, recursionCount + 1);
     }
 }
 
@@ -255,25 +265,20 @@ int Octree::readElementData(OctreeElement* destinationElement, const unsigned ch
 
     int bytesLeftToRead = bytesAvailable;
     int bytesRead = 0;
-    bool wantDebug = false;
 
     // give this destination element the child mask from the packet
     const unsigned char ALL_CHILDREN_ASSUMED_TO_EXIST = 0xFF;
     
     if ((size_t)bytesLeftToRead < sizeof(unsigned char)) {
-        if (wantDebug) {
+        qDebug() << "UNEXPECTED: readElementData() only had " << bytesLeftToRead << " bytes. "
-            qDebug() << "UNEXPECTED: readElementData() only had " << bytesLeftToRead << " bytes. "
+                    "Not enough for meaningful data.";
-                        "Not enough for meaningful data.";
-        }
         return bytesAvailable; // assume we read the entire buffer...
     }
     
     if (destinationElement->getScale() < SCALE_AT_DANGEROUSLY_DEEP_RECURSION) {
-        if (wantDebug) {
+        qDebug() << "UNEXPECTED: readElementData() destination element is unreasonably small [" 
-            qDebug() << "UNEXPECTED: readElementData() destination element is unreasonably small [" 
+                << destinationElement->getScale() * (float)TREE_SCALE << " meters] "
-                    << destinationElement->getScale() * (float)TREE_SCALE << " meters] "
+                << " Discarding " << bytesAvailable << " remaining bytes.";
-                    << " Discarding " << bytesAvailable << " remaining bytes.";
-        }
         return bytesAvailable; // assume we read the entire buffer...
     }
     
@@ -322,7 +327,7 @@ int Octree::readElementData(OctreeElement* destinationElement, const unsigned ch
                                                 : sizeof(childInBufferMask);
 
     if (bytesLeftToRead < bytesForMasks) {
-        if (wantDebug) {
+        if (bytesLeftToRead > 0) {
             qDebug() << "UNEXPECTED: readElementDataFromBuffer() only had " << bytesLeftToRead << " bytes before masks. "
                         "Not enough for meaningful data.";
         }
@@ -385,7 +390,6 @@ void Octree::readBitstreamToTree(const unsigned char * bitstream, unsigned long
                                     ReadBitstreamToTreeParams& args) {
     int bytesRead = 0;
     const unsigned char* bitstreamAt = bitstream;
-    bool wantDebug = false;
 
     // If destination element is not included, set it to root
     if (!args.destinationElement) {
@@ -398,14 +402,24 @@ void Octree::readBitstreamToTree(const unsigned char * bitstream, unsigned long
 
     while (bitstreamAt < bitstream + bufferSizeBytes) {
         OctreeElement* bitstreamRootElement = nodeForOctalCode(args.destinationElement, (unsigned char *)bitstreamAt, NULL);
-
         int numberOfThreeBitSectionsInStream = numberOfThreeBitSectionsInCode(bitstreamAt, bufferSizeBytes);
+        if (numberOfThreeBitSectionsInStream > UNREASONABLY_DEEP_RECURSION) {
+            static QString repeatedMessage
+                = LogHandler::getInstance().addRepeatedMessageRegex(
+                        "UNEXPECTED: parsing of the octal code would make UNREASONABLY_DEEP_RECURSION... "
+                        "numberOfThreeBitSectionsInStream: \\d+ This buffer is corrupt. Returning."
+                    );
+
+
+            qDebug() << "UNEXPECTED: parsing of the octal code would make UNREASONABLY_DEEP_RECURSION... "
+                        "numberOfThreeBitSectionsInStream:" << numberOfThreeBitSectionsInStream <<
+                        "This buffer is corrupt. Returning.";
+            return;
+        }
         
         if (numberOfThreeBitSectionsInStream == OVERFLOWED_OCTCODE_BUFFER) {
-            if (wantDebug) {
+            qDebug() << "UNEXPECTED: parsing of the octal code would overflow the buffer. "
-                qDebug() << "UNEXPECTED: parsing of the octal code would overflow the buffer. "
+                        "This buffer is corrupt. Returning.";
-                            "This buffer is corrupt. Returning.";
-            }
             return;
         }
         

libraries/octree/src/Octree.h

@@ -373,7 +373,7 @@ protected:
     static bool countOctreeElementsOperation(OctreeElement* element, void* extraData);
 
     OctreeElement* nodeForOctalCode(OctreeElement* ancestorElement, const unsigned char* needleCode, OctreeElement** parentOfFoundElement) const;
-    OctreeElement* createMissingElement(OctreeElement* lastParentElement, const unsigned char* codeToReach);
+    OctreeElement* createMissingElement(OctreeElement* lastParentElement, const unsigned char* codeToReach, int recursionCount = 0);
     int readElementData(OctreeElement *destinationElement, const unsigned char* nodeData,
                 int bufferSizeBytes, ReadBitstreamToTreeParams& args);
 

libraries/octree/src/OctreeConstants.h

@@ -35,10 +35,11 @@ const int MAX_TREE_SLICE_BYTES = 26;
 const float VIEW_FRUSTUM_FOV_OVERSEND = 60.0f;
 
 // These are guards to prevent our voxel tree recursive routines from spinning out of control
-const int UNREASONABLY_DEEP_RECURSION = 20; // use this for something that you want to be shallow, but not spin out
+const int UNREASONABLY_DEEP_RECURSION = 29; // use this for something that you want to be shallow, but not spin out
 const int DANGEROUSLY_DEEP_RECURSION = 200; // use this for something that needs to go deeper
 const float SCALE_AT_UNREASONABLY_DEEP_RECURSION = (1.0f / powf(2.0f, UNREASONABLY_DEEP_RECURSION));
 const float SCALE_AT_DANGEROUSLY_DEEP_RECURSION = (1.0f / powf(2.0f, DANGEROUSLY_DEEP_RECURSION));
+const float SMALLEST_REASONABLE_OCTREE_ELEMENT_SCALE = SCALE_AT_UNREASONABLY_DEEP_RECURSION * 2.0f; // 0.00006103515 meter ~1/10,0000th
 
 const int DEFAULT_MAX_OCTREE_PPS = 600; // the default maximum PPS we think any octree based server should send to a client
 

libraries/octree/src/OctreeElement.h

@@ -36,8 +36,6 @@ class ReadBitstreamToTreeParams;
 class Shape;
 class VoxelSystem;
 
-const float SMALLEST_REASONABLE_OCTREE_ELEMENT_SCALE = (1.0f / TREE_SCALE) / 10000.0f; // 1/10,000th of a meter
-
 // Callers who want delete hook callbacks should implement this class
 class OctreeElementDeleteHook {
 public: