Add protection from some network issues

src/factory/PVDataCreateFactory.cpp

@@ -409,23 +409,19 @@ template<>
 void PVValueArray<string>::deserialize(ByteBuffer *pbuffer,
         DeserializableControl *pcontrol) {
 
-    size_t size = this->getArray()->getArraySizeType() == Array::fixed ?
-                this->getArray()->getMaximumCapacity() :
-                SerializeHelper::readSize(pbuffer, pcontrol);
-
-    svector nextvalue(thaw(value));
-
-    // Decide if we must re-allocate
-    if(size > nextvalue.size() || !nextvalue.unique())
-        nextvalue.resize(size);
-    else if(size < nextvalue.size())
-        nextvalue.slice(0, size);
-
-
-    string * pvalue = nextvalue.data();
-    for(size_t i = 0; i<size; i++) {
-        pvalue[i] = SerializeHelper::deserializeString(pbuffer,
-                                                       pcontrol);
+    const size_t size = this->getArray()->getArraySizeType() == Array::fixed ?
+        this->getArray()->getMaximumCapacity() :
+        SerializeHelper::readSize(pbuffer, pcontrol);
+
+    svector nextvalue(thaw(value)); // Reuse old storage
+
+    // Don't trust 'size' for new or larger arrays, start
+    // small and grow them as elements are deserialized.
+    nextvalue.resize(0);
+    nextvalue.reserve(size < 64 ? size : 64);
+    for (size_t i = 0; i<size; i++) {
+        nextvalue.push_back(
+            SerializeHelper::deserializeString(pbuffer, pcontrol));
     }
     value = freeze(nextvalue);
     // inform about the change?

src/factory/PVStructureArray.cpp

@@ -160,28 +160,28 @@ void PVStructureArray::deserialize(ByteBuffer *pbuffer,
         DeserializableControl *pcontrol) {
     svector data(reuse());
 
-    size_t size = this->getArray()->getArraySizeType() == Array::fixed ?
-                this->getArray()->getMaximumCapacity() :
-                SerializeHelper::readSize(pbuffer, pcontrol);
-
-    data.resize(size);
+    const size_t size = this->getArray()->getArraySizeType() == Array::fixed ?
+        this->getArray()->getMaximumCapacity() :
+        SerializeHelper::readSize(pbuffer, pcontrol);
 
     StructureConstPtr structure = structureArray->getStructure();
 
     PVDataCreatePtr pvDataCreate = getPVDataCreate();
 
+    // Don't trust 'size' for new or larger arrays, start
+    // small and grow them as elements are deserialized.
+    data.resize(0);
+    data.reserve(size < 64 ? size : 64);
     for(size_t i = 0; i<size; i++) {
         pcontrol->ensureData(1);
         size_t temp = pbuffer->getByte();
-        if(temp==0) {
-            data[i].reset();
-        }
-        else {
-            if(data[i].get()==NULL || !data[i].unique()) {
-                data[i] = pvDataCreate->createPVStructure(structure);
-            }
-            data[i]->deserialize(pbuffer, pcontrol);
+
+        PVStructurePtr pvStructure;
+        if(temp!=0) {
+            pvStructure = pvDataCreate->createPVStructure(structure);
+            pvStructure->deserialize(pbuffer, pcontrol);
         }
+        data.push_back(pvStructure);
     }
     replace(freeze(data)); // calls postPut()
 }

src/factory/PVUnion.cpp

@@ -158,21 +158,26 @@ void PVUnion::deserialize(ByteBuffer *pbuffer, DeserializableControl *pcontrol)
     }
     else
     {
-        int32 previousSelector = selector;
-        selector = static_cast<int32>(SerializeHelper::readSize(pbuffer, pcontrol));
-        if (selector != UNDEFINED_INDEX)
+        int32 index = static_cast<int32>(SerializeHelper::readSize(pbuffer, pcontrol));
+        if (index == UNDEFINED_INDEX)
         {
-            if (selector != previousSelector)
+            selector = index;
+            value.reset();
+        }
+        else if (index < 0 || size_t(index) >= unionPtr->getFields().size())
+            throw std::runtime_error("union index out of bounds");
+        else
+        {
+            if (selector != index)
             {
-                FieldConstPtr field = unionPtr->getField(selector);
+                selector = index;
+                FieldConstPtr field = unionPtr->getField(index);
                 // try to reuse existing field instance
                 if (!value.get() || *value->getField() != *field)
                     value = pvDataCreate->createPVField(field);
             }
             value->deserialize(pbuffer, pcontrol);
         }
-        else
-            value.reset();
     }
 }
 

src/factory/PVUnionArray.cpp

@@ -159,28 +159,28 @@ void PVUnionArray::deserialize(ByteBuffer *pbuffer,
         DeserializableControl *pcontrol) {
     svector data(reuse());
 
-    size_t size = this->getArray()->getArraySizeType() == Array::fixed ?
-                this->getArray()->getMaximumCapacity() :
-                SerializeHelper::readSize(pbuffer, pcontrol);
-
-    data.resize(size);
+    const size_t size = this->getArray()->getArraySizeType() == Array::fixed ?
+        this->getArray()->getMaximumCapacity() :
+        SerializeHelper::readSize(pbuffer, pcontrol);
 
     UnionConstPtr punion = unionArray->getUnion();
 
     PVDataCreatePtr pvDataCreate = getPVDataCreate();
 
+    // Don't trust 'size' for new or larger arrays, start
+    // small and grow them as elements are deserialized.
+    data.resize(0);
+    data.reserve(size < 64 ? size : 64);
     for(size_t i = 0; i<size; i++) {
         pcontrol->ensureData(1);
         size_t temp = pbuffer->getByte();
-        if(temp==0) {
-            data[i].reset();
-        }
-        else {
-            if(data[i].get()==NULL || !data[i].unique()) {
-                data[i] = pvDataCreate->createPVUnion(punion);
-            }
-            data[i]->deserialize(pbuffer, pcontrol);
+
+        PVUnionPtr pvUnion;
+        if(temp!=0) {
+            pvUnion = pvDataCreate->createPVUnion(punion);
+            pvUnion->deserialize(pbuffer, pcontrol);
         }
+        data.push_back(pvUnion);
     }
     replace(freeze(data)); // calls postPut()
 }

src/misc/bitSet.cpp

@@ -337,17 +337,15 @@ namespace epics { namespace pvData {
     }
 
     void BitSet::deserialize(ByteBuffer* buffer, DeserializableControl* control) {
+        size_t size = SerializeHelper::readSize(buffer, control);
+        uint32 bytes = (size == (size_t) -1) ? 0 : static_cast<uint32>(size);
 
-        uint32 bytes = static_cast<uint32>(SerializeHelper::readSize(buffer, control)); // in bytes
+        // Validate availability before allocating storage
+        control->ensureData(bytes);
 
         size_t wordsInUse = (bytes + 7) / BYTES_PER_WORD;
         words.resize(wordsInUse);
 
-        if (wordsInUse == 0)
-            return;
-
-        control->ensureData(bytes);
-
         uint32 i = 0;
         uint32 longs = bytes / 8;
         while (i < longs)

src/misc/pv/serializeHelper.h

@@ -50,7 +50,7 @@ namespace epics {
              *
              * @param[in] buffer deserialization buffer.
              * @param[in] control the DeserializableControl.
-             * @returns array size.
+             * @returns array size, or (size_t) -1.
              */
             static std::size_t readSize(ByteBuffer* buffer,
                     DeserializableControl* control);
@@ -80,20 +80,18 @@ namespace epics {
 
             /**
              * std::string deserialization helper method.
-             * TODO This method cannot return "null", but Java implementation
-             * could have serialized "null" value as well. We need to decide
-             * how to deserialize "null".
              *
              * @param[in] buffer deserialization buffer
              * @param[in] control control
+             * @param[in] max length limit
              * @returns deserialized string
              *
              * @todo This method cannot return "null", but Java implementation
-             * could have serialized "null" value as well. We need to decide
-             * how to deserialize "null".
+             * could have serialized "null" value as well. This implementation
+             * returns an empty string in that event.
              */
             static std::string deserializeString(ByteBuffer* buffer,
-                    DeserializableControl* control);
+                    DeserializableControl* control, std::size_t max = INT32_MAX);
 
         private:
             SerializeHelper() {};
@@ -112,4 +110,6 @@ namespace epics {
     }
 }
 
+#define DESERIALIZE_STRING_HAS_LIMIT
+
 #endif /* SERIALIZEHELPER_H_ */

src/misc/serializeHelper.cpp

@@ -47,12 +47,13 @@ namespace epics {
                 DeserializableControl* control) {
             control->ensureData(1);
             int8 b = buffer->getByte();
-            if(b==-1)
-                return -1;
-            else if(b==-2) {
+            if (b == -1)
+                return -1;  // NULL
+            else if (b == -2) {
                 control->ensureData(sizeof(int32));
                 int32 s = buffer->getInt();
-                if(s<0) THROW_BASE_EXCEPTION("negative size");
+                if (s < 0)
+                    THROW_BASE_EXCEPTION("negative size");
                 return s;
             }
             else
@@ -100,44 +101,37 @@ namespace epics {
         }
 
         string SerializeHelper::deserializeString(ByteBuffer* buffer,
-                DeserializableControl* control) {
+                DeserializableControl* control, std::size_t max) {
 
             std::size_t size = SerializeHelper::readSize(buffer, control);
-            if(size!=(size_t)-1)    // TODO null strings check, to be removed in the future
-            {
-                if (buffer->getRemaining()>=size)
-                {
-                    // entire string is in buffer, simply create a string out of it (copy)
-                    std::size_t pos = buffer->getPosition();
-                    string str(buffer->getBuffer()+pos, size);
-                    buffer->setPosition(pos+size);
-                    return str;
-                }
+            if (size == (std::size_t) -1)
+                return std::string();   // NULL from old implementations
+
+            if (size > max)
+                THROW_BASE_EXCEPTION("string too long");
+
+            if (buffer->getRemaining() >= size) { // All present
+                std::size_t pos = buffer->getPosition();
+                string str(buffer->getBuffer() + pos, size);
+                buffer->setPosition(pos + size);
+                return str;
+            }
+
+            string str;
+            str.reserve(size);
+            std::size_t i = 0;
+            while (true) {
+                std::size_t toRead = min(size-i, buffer->getRemaining());
+                std::size_t pos = buffer->getPosition();
+                str.append(buffer->getBuffer() + pos, toRead);
+                buffer->setPosition(pos + toRead);
+                i += toRead;
+                if (i < size)
+                    control->ensureData(1); // at least one
                 else
-                {
-                    string str;
-                    str.reserve(size);
-                    try {
-                        std::size_t i = 0;
-                        while(true) {
-                            std::size_t toRead = min(size-i, buffer->getRemaining());
-                            std::size_t pos = buffer->getPosition();
-                            str.append(buffer->getBuffer()+pos, toRead);
-                            buffer->setPosition(pos+toRead);
-                            i += toRead;
-                            if(i<size)
-                                control->ensureData(1); // at least one
-                            else
-                                break;
-                        }
-                        return str;
-                    } catch(...) {
-                        throw;
-                    }
-                }
+                    break;
             }
-            else
-                return std::string();
+            return str;
         }
     }
 }

src/misc/status.cpp

@@ -72,6 +72,14 @@ void Status::deserialize(ByteBuffer *buffer, DeserializableControl *flusher)
            m_stackDump.clear();
        }
     }
+    else if (typeCode < 0 || typeCode > STATUSTYPE_FATAL)
+    {
+        // Leave *this as fatal and throw
+        m_statusType = STATUSTYPE_FATAL;
+        m_message = "deserialized type code was invalid";
+        m_stackDump.clear();
+        THROW_BASE_EXCEPTION("invalid Status type code");
+    }
     else
     {
         m_statusType = (StatusType)typeCode;

testApp/misc/testBitSet.cpp

@@ -303,15 +303,57 @@ static void testSerialize()
 #undef TOFRO
 }
 
+static void testDeserializeOversize()
+{
+    testDiag("testDeserializeOversize");
+
+    // Size field declares INT32_MAX bytes but no payload follows: 0xFE
+    // long-form marker + big-endian int32. deserialize() must reject this
+    // via ensureData() before allocating a ~2GB backing store.
+    const char msg[] = {(char)0xFE, 0x7F, (char)0xFF, (char)0xFF, (char)0xFF};
+    BitSet dut;
+    ByteBuffer buf((char*)msg, sizeof(msg), EPICS_ENDIAN_BIG);
+    try {
+        deserializeFromBuffer(&dut, buf);
+        testFail("oversized BitSet size was not rejected");
+    } catch(std::exception& e) {
+        testPass("oversized BitSet size rejected: %s", e.what());
+    }
+}
+
+static void testDeserializeNull()
+{
+    testDiag("testDeserializeNull");
+
+    // A NULL size is encoded on the wire as the single byte 0xFF. Without
+    // the null check in deserialize() this would be cast to UINT32_MAX bytes
+    // and overflow the allocation size; it must instead yield an empty BitSet.
+    const char msg[] = {(char)0xFF};
+    BitSet dut;
+    dut.set(5); // ensure deserialize actually clears/replaces existing bits
+    ByteBuffer buf((char*)msg, sizeof(msg), EPICS_ENDIAN_BIG);
+    try {
+        deserializeFromBuffer(&dut, buf);
+        testPass("NULL BitSet size accepted");
+    } catch(std::exception& e) {
+        testFail("NULL BitSet size rejected: %s", e.what());
+    }
+    testOk1(dut.isEmpty());
+    testOk(buf.getRemaining()==0, "buffer remaining 0 == %u",
+        (unsigned)buf.getRemaining());
+}
+
 } // namespace
 
 MAIN(testBitSet)
 {
-    testPlan(90);
+    testPlan(94);
     testInitialize();
     testGetSetClearFlip();
     testOperators();
     testLogical();
     testSerialize();
+    testDeserializeOversize();
+    testDeserializeNull();
     return testDone();
 }