Home Help Center

Questions about sanity-check of sequence sizes during sequence unmarshaling

rc_hzrc_hz Member Eric RCOrganization: www.genband.comProject: No project yet ✭✭✭
I have noticed the comments in src/ice/BasicStream.cpp
//
// startSeq() and endSeq() sanity-check sequence sizes during
// unmarshaling and prevent malicious messages with incorrect sequence
// sizes from causing the receiver to use up all available memory by
// allocating sequences with an impossibly large number of elements.
//
// The code generator inserts calls to startSeq() and endSeq() around
// the code to unmarshal a sequence of a variable-length type. startSeq()
// is called immediately after reading the sequence size, and endSeq() is
// called after reading the final element of a sequence.
//
// For a sequence of a fixed-length type, the code generator inserts a
// call to checkFixedSeq(), which does not cause any memory allocations.
//
// For sequences that contain constructed types that, in turn, contain
// sequences, the code generator also inserts a call to endElement()
// (inlined in BasicStream.h) after unmarshaling each element.
//
// startSeq() is passed the unmarshaled element count, plus the
// minimum size (in bytes) occupied by the sequence's element
// type. numElements * minSize is the smallest possible number of
// bytes that the sequence will occupy on the wire.
//
// Every time startSeq() is called, it pushes the element count and
// the minimum size on a stack. Every time endSeq() is called, it pops
// the stack.
//
// For an ordinary sequence (one that does not (recursively) contain
// nested sequences), numElements * minSize must be less than the
// number of bytes remaining in the stream.
//
// For a sequence that is nested within some other sequence, there
// must be enough bytes remaining in the stream for this sequence
// (numElements + minSize), plus the sum of the bytes required by the
// remaining elements of all the enclosing sequences.
//
// For the enclosing sequences, numElements - 1 is the number of
// elements for which unmarshaling has not started yet. (The call to
// endElement() in the generated code decrements that number whenever
// a sequence element is unmarshaled.)
//
// For sequences that have variable-length elements
    , checkSeq() is called
    // whenever an element is unmarshaled. checkSeq() also checks whether
    // the stream has a sufficient number of bytes remaining. This means
    // that, for messages with bogus sequence sizes, unmarshaling is
    // aborted at the earliest possible point.
    //

    We know that Ice has defined the following TYPE hierarcy(Chapter 4.8/4.9)
    1)Basic Slice Types
    bool/byte/short/int/long/float/double/string

    2)User-defined Types
    enumeration/structure/sequence/dictionary/

    3)class

    However, some new concepts appeared in the above quote:
    1)a sequence of a variable-length type
    2)sequences that contain constructed types
    3)a sequence of a fixed-length type
    But I can not find the accurate definitions of "variable-length type" or "constructed type" or "fixed-length type" in Ice's manual. Just one example: If a struct has one member of class type, what's its type ?


    Another question:
    // startSeq() is passed the unmarshaled element count, plus the
    // minimum size (in bytes) occupied by the sequence's element
    // type.
    How to know the minimum size of a the sequence's element type?

    I have noticed the following:
    -- test/ice/stream/Test.cpp (generated by Test.ice)
    void
    Test::__read(::IceInternal::BasicStream* __is, ::Test::MyClassS& v, ::Test::__U__MyClassS)
    {
        ::Ice::Int sz;
        __is->readSize(sz);
    [COLOR=Blue]    __is->startSeq(sz, 4);		//4 bytes [/COLOR]     v.resize(sz);
        for(int i = 0; i < sz; ++i)
        {
    		__is->read(::Test::__patch__MyClassPtr, &v[i]);
    		__is->checkSeq();
    		__is->endElement();
        }
        __is->endSeq(sz);
    }
    
    -- test/ice/operations/Test.cpp (generated by Test.ice)
    void
    Test::__read(::IceInternal::BasicStream* __is, ::Test::MyClassS& v, ::Test::__U__MyClassS)
    {
        ::Ice::Int sz;
        __is->readSize(sz);
    [COLOR=Blue]    __is->startSeq(sz, 2);		//2 bytes[/COLOR]    v.resize(sz);
        for(int i = 0; i < sz; ++i)
        {
    		::Test::__read(__is, v[i]);
    		__is->checkSeq();
    		__is->endElement();
        }
        __is->endSeq(sz);
    }
    
    In my understanding, a sequence of Class's minimal element size should be 4 bytes because a Class's Identity is 4 bytes(Int) when marshalling. But why it is just 2 bytes in test/ice/operations/Test.cpp ?

    Thanks.

    Comments

    • michimichi Member Michi HenningOrganization: Triodia TechnologiesProject: I have a passing interest in Ice :-) ✭✭✭
      But I can not find the accurate definitions of "variable-length type" or "constructed type" or "fixed-length type" in Ice's manual. Just one example: If a struct has one member of class type, what's its type ?

      A fixed-length type is a type whose values have a size that is known at compile time, such as bool.

      A constructed type is a class, struct, sequence, dictionary, or enum type.

      A variable-length type is a type whose values have sizes that cannot be known at compile time.

      The following rules apply:
      • bool, byte, short, int, long, float, and double are fixed-length types.
      • string, proxies, Object, and types derived from Object are variable-length types.
      • Sequences and dictionaries are variable-length types.
      • Constructed types are fixed-length if they, recursively, only contain fixed-length types.
      How to know the minimum size of a the sequence's element type?

      This code uses the minimum on-the-wire size of the data to determine whether it is possible for the remainder of a request to be unmarshaled within the limits of the request size that was sent in the header. The point of all this is to prevent someone from sending malformed packets with impossibly large sizes and to cause an Ice process to allocate impossibly large amounts of memory during unmarshaling.

      In the operations test, MyClassS is a sequence of proxies. The minimum on-the-wire size of a proxy is two bytes for a nil proxy (empty name and empty category).

      In the stream test, MyClassS is a sequence of classes, not a sequence of proxies, so a different minimum size applies.

      Cheers,

      Michi.
    • rc_hzrc_hz Member Eric RCOrganization: www.genband.comProject: No project yet ✭✭✭
      michi wrote:
      • bool, byte, short, int, long, float, and double are fixed-length types.
      • string, proxies, Object, and types derived from Object are variable-length types.
      • Sequences and dictionaries are variable-length types.
      • Constructed types are fixed-length if they, recursively, only contain fixed-length types.

      Thank you, Michi.

      How about enum type ? It's a construct type and it contains only fixed-length types, so it is a fixed-length type, right ?
    • rc_hzrc_hz Member Eric RCOrganization: www.genband.comProject: No project yet ✭✭✭
      A bug ???
      -- src/Ice/BasicStream.cpp
      void
      IceInternal::BasicStream::startSeq(int numElements, int minSize)
      {
          if(numElements == 0) // Optimization to avoid pushing a useless stack frame.
          {
      		return;
          }
      
          //
          // Push the current sequence details on the stack.
          //
          SeqData* sd = new SeqData(numElements, minSize);
          sd->previous = _seqDataStack;
          _seqDataStack = sd;
      
          int bytesLeft = static_cast<int>(b.end() - i);
          [COLOR=Red]if(_seqDataStack == 0) // Outermost sequence[/COLOR]
          { [COLOR=Blue]//This is always false[/COLOR]
      		//
      		// The sequence must fit within the message.
      		//
      		if(numElements * minSize > bytesLeft) 
      		{
      		    throw UnmarshalOutOfBoundsException(__FILE__, __LINE__);
      		}
          }
          else // Nested sequence
          {
      		checkSeq(bytesLeft);
          }
      }
      

      Maybe the red part should be replaced:
          [COLOR=Red]if (sd->previous == 0) // Outermost sequence[/COLOR]
      
    • michimichi Member Michi HenningOrganization: Triodia TechnologiesProject: I have a passing interest in Ice :-) ✭✭✭
      Thanks very much for pointing this out! I've fixed this for the next release.
    Sign In or Register to comment.