Skip to content

Commit

Permalink
Fix reading index packets (#249)
Browse files Browse the repository at this point in the history 
It would throw an exception in IndexPacket::verify() when reading them because it was checking the wrong size.

- split index packet header out so we can check against that size instead (new line 582)
- add extra context to index and data packet exceptions to make them easier to track down

I guess nobody uses index packets...
  • Loading branch information
@asmaloney
asmaloney committed May 20, 2023
1 parent 458b489 commit 6330aa1
Showing 1 changed file with 73 additions and 55 deletions.
128 changes: 73 additions & 55 deletions src/Packet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,22 @@

using namespace e57;

struct IndexPacket
struct IndexPacketHeader
{
static constexpr unsigned MAX_ENTRIES = 2048;

const uint8_t packetType = INDEX_PACKET;

uint8_t packetFlags = 0; // flag bitfields
uint16_t packetLogicalLengthMinus1 = 0;
uint16_t entryCount = 0;
uint8_t indexLevel = 0;
uint8_t reserved1[9] = {}; // must be zero
};

struct IndexPacket
{
IndexPacketHeader header;

static constexpr unsigned MAX_ENTRIES = 2048;

struct IndexPacketEntry
{
Expand Down Expand Up @@ -351,7 +356,8 @@ void DataPacketHeader::verify( unsigned bufferLength ) const
// Verify that packet is correct type
if ( packetType != DATA_PACKET )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "packetType=" + toString( packetType ) );
throw E57_EXCEPTION2( ErrorBadCVPacket,
"expected Data; packetType=" + toString( packetType ) );
}

// ??? check reserved flags zero?
Expand All @@ -360,34 +366,35 @@ void DataPacketHeader::verify( unsigned bufferLength ) const
unsigned packetLength = packetLogicalLengthMinus1 + 1;
if ( packetLength < sizeof( *this ) )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "packetLength=" + toString( packetLength ) );
throw E57_EXCEPTION2( ErrorBadCVPacket, "DATA; packetLength=" + toString( packetLength ) );
}

// Check packet length is multiple of 4
if ( packetLength % 4 )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "packetLength=" + toString( packetLength ) );
throw E57_EXCEPTION2( ErrorBadCVPacket, "DATA; packetLength=" + toString( packetLength ) );
}

// Check actual packet length is large enough.
if ( bufferLength > 0 && packetLength > bufferLength )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "packetLength=" + toString( packetLength ) +
throw E57_EXCEPTION2( ErrorBadCVPacket, "DATA; packetLength=" + toString( packetLength ) +
" bufferLength=" + toString( bufferLength ) );
}

// Make sure there is at least one entry in packet ??? 0 record cvect
// allowed?
if ( bytestreamCount == 0 )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "bytestreamCount=" + toString( bytestreamCount ) );
throw E57_EXCEPTION2( ErrorBadCVPacket,
"DATA; bytestreamCount=" + toString( bytestreamCount ) );
}

// Check packet is at least long enough to hold bytestreamBufferLength array
if ( ( sizeof( DataPacketHeader ) + 2 * bytestreamCount ) > packetLength )
{
throw E57_EXCEPTION2( ErrorBadCVPacket,
"packetLength=" + toString( packetLength ) +
"DATA; packetLength=" + toString( packetLength ) +
" bytestreamCount=" + toString( bytestreamCount ) );
}
}
Expand Down Expand Up @@ -444,16 +451,16 @@ void DataPacket::verify( unsigned bufferLength ) const
// If needed is not with 3 bytes of actual packet size, have an error
if ( needed > packetLength || needed + 3 < packetLength )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "needed=" + toString( needed ) +
"packetLength=" + toString( packetLength ) );
throw E57_EXCEPTION2( ErrorBadCVPacket, "DATA; needed=" + toString( needed ) +
" packetLength=" + toString( packetLength ) );
}

// Verify that padding at end of packet is zero
for ( unsigned i = needed; i < packetLength; i++ )
{
if ( reinterpret_cast<const char *>( this )[i] != 0 )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "i=" + toString( i ) );
throw E57_EXCEPTION2( ErrorBadCVPacket, "DATA; i=" + toString( i ) );
}
}
}
Expand Down Expand Up @@ -564,57 +571,64 @@ void IndexPacket::verify( unsigned bufferLength, uint64_t totalRecordCount,
// file version#?

// Verify that packet is correct type
if ( packetType != INDEX_PACKET )
if ( header.packetType != INDEX_PACKET )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "packetType=" + toString( packetType ) );
throw E57_EXCEPTION2( ErrorBadCVPacket,
"expected Index; packetType=" + toString( header.packetType ) );
}

// Check packetLength is at least large enough to hold header
unsigned packetLength = packetLogicalLengthMinus1 + 1;
if ( packetLength < sizeof( *this ) )
unsigned packetLength = header.packetLogicalLengthMinus1 + 1;
if ( packetLength < sizeof( IndexPacketHeader ) )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "packetLength=" + toString( packetLength ) );
throw E57_EXCEPTION2( ErrorBadCVPacket,
"INDEX; less than size of IndexPacketHeader; packetLength=" +
toString( packetLength ) );
}

// Check packet length is multiple of 4
if ( packetLength % 4 )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "packetLength=" + toString( packetLength ) );
throw E57_EXCEPTION2( ErrorBadCVPacket, "INDEX; length not multiple of 4; packetLength=" +
toString( packetLength ) );
}

// Make sure there is at least one entry in packet ??? 0 record cvect
// allowed?
if ( entryCount == 0 )
if ( header.entryCount == 0 )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "entryCount=" + toString( entryCount ) );
throw E57_EXCEPTION2( ErrorBadCVPacket,
"INDEX; entryCount=" + toString( header.entryCount ) );
}

// Have to have <= 2048 entries
if ( entryCount > MAX_ENTRIES )
if ( header.entryCount > MAX_ENTRIES )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "entryCount=" + toString( entryCount ) );
throw E57_EXCEPTION2( ErrorBadCVPacket,
"INDEX; entryCount=" + toString( header.entryCount ) );
}

// Index level should be <= 5. Because (5+1)* 11 bits = 66 bits, which will cover largest number
// of chunks possible.
if ( indexLevel > 5 )
if ( header.indexLevel > 5 )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "indexLevel=" + toString( indexLevel ) );
throw E57_EXCEPTION2( ErrorBadCVPacket,
"INDEX; indexLevel=" + toString( header.indexLevel ) );
}

// Index packets above level 0 must have at least two entries (otherwise no point to existing).
//??? check that this is in spec
if ( indexLevel > 0 && entryCount < 2 )
if ( header.indexLevel > 0 && header.entryCount < 2 )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "indexLevel=" + toString( indexLevel ) +
" entryCount=" + toString( entryCount ) );
throw E57_EXCEPTION2( ErrorBadCVPacket, "INDEX; indexLevel=" + toString( header.indexLevel ) +
" entryCount=" + toString( header.entryCount ) );
}

// If not later version, verify reserved fields are zero. ??? test file
// version if (version <= E57_FORMAT_MAJOR) { //???
for ( unsigned i = 0; i < sizeof( reserved1 ); i++ )
for ( unsigned i = 0; i < sizeof( header.reserved1 ); i++ )
{
if ( reserved1[i] != 0 )
if ( header.reserved1[i] != 0 )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "i=" + toString( i ) );
}
Expand All @@ -623,64 +637,67 @@ void IndexPacket::verify( unsigned bufferLength, uint64_t totalRecordCount,
// Check actual packet length is large enough.
if ( bufferLength > 0 && packetLength > bufferLength )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "packetLength=" + toString( packetLength ) +
throw E57_EXCEPTION2( ErrorBadCVPacket, "INDEX; packetLength=" + toString( packetLength ) +
" bufferLength=" + toString( bufferLength ) );
}

// Check if entries will fit in space provided
unsigned neededLength = 16 + 8 * entryCount;
if ( packetLength < neededLength )
const unsigned cNeededLength =
sizeof( IndexPacketHeader ) + sizeof( IndexPacketEntry ) * header.entryCount;
if ( packetLength < cNeededLength )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "packetLength=" + toString( packetLength ) +
" neededLength=" + toString( neededLength ) );
throw E57_EXCEPTION2( ErrorBadCVPacket, "INDEX; packetLength=" + toString( packetLength ) +
" neededLength=" + toString( cNeededLength ) );
}

#if VALIDATE_DEEP
// Verify padding at end is zero.
const char *p = reinterpret_cast<const char *>( this );
for ( unsigned i = neededLength; i < packetLength; i++ )
for ( unsigned i = cNeededLength; i < packetLength; i++ )
{
if ( p[i] != 0 )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "i=" + toString( i ) );
throw E57_EXCEPTION2( ErrorBadCVPacket, "INDEX; padding; i=" + toString( i ) );
}
}

// Verify records and offsets are in sorted order
for ( unsigned i = 0; i < entryCount; i++ )
for ( unsigned i = 0; i < header.entryCount; i++ )
{
// Check chunkRecordNumber is in bounds
if ( totalRecordCount > 0 && entries[i].chunkRecordNumber >= totalRecordCount )
{
throw E57_EXCEPTION2( ErrorBadCVPacket,
"i=" + toString( i ) +
"INDEX; record# in bounds; i=" + toString( i ) +
" chunkRecordNumber=" + toString( entries[i].chunkRecordNumber ) +
" totalRecordCount=" + toString( totalRecordCount ) );
}

// Check record numbers are strictly increasing
if ( i > 0 && entries[i - 1].chunkRecordNumber >= entries[i].chunkRecordNumber )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "i=" + toString( i ) + " prevChunkRecordNumber=" +
toString( entries[i - 1].chunkRecordNumber ) +
" currentChunkRecordNumber=" +
toString( entries[i].chunkRecordNumber ) );
throw E57_EXCEPTION2(
ErrorBadCVPacket,
"INDEX; record numbers increasing; i=" + toString( i ) +
" prevChunkRecordNumber=" + toString( entries[i - 1].chunkRecordNumber ) +
" currentChunkRecordNumber=" + toString( entries[i].chunkRecordNumber ) );
}

// Check chunkPhysicalOffset is in bounds
if ( fileSize > 0 && entries[i].chunkPhysicalOffset >= fileSize )
{
throw E57_EXCEPTION2( ErrorBadCVPacket, "i=" + toString( i ) + " chunkPhysicalOffset=" +
toString( entries[i].chunkPhysicalOffset ) +
" fileSize=" + toString( fileSize ) );
throw E57_EXCEPTION2(
ErrorBadCVPacket,
"INDEX; physical offset in bounds; i=" + toString( i ) + " chunkPhysicalOffset=" +
toString( entries[i].chunkPhysicalOffset ) + " fileSize=" + toString( fileSize ) );
}

// Check chunk offsets are strictly increasing
if ( i > 0 && entries[i - 1].chunkPhysicalOffset >= entries[i].chunkPhysicalOffset )
{
throw E57_EXCEPTION2(
ErrorBadCVPacket,
"i=" + toString( i ) +
"INDEX; chunk offsets increasing; i=" + toString( i ) +
" prevChunkPhysicalOffset=" + toString( entries[i - 1].chunkPhysicalOffset ) +
" currentChunkPhysicalOffset=" + toString( entries[i].chunkPhysicalOffset ) );
}
Expand All @@ -691,25 +708,26 @@ void IndexPacket::verify( unsigned bufferLength, uint64_t totalRecordCount,
#ifdef E57_ENABLE_DIAGNOSTIC_OUTPUT
void IndexPacket::dump( int indent, std::ostream &os ) const
{
os << space( indent ) << "packetType: " << static_cast<unsigned>( packetType )
<< std::endl;
os << space( indent ) << "packetFlags: " << static_cast<unsigned>( packetFlags )
os << space( indent )
<< "packetType: " << static_cast<unsigned>( header.packetType ) << std::endl;
os << space( indent )
<< "packetFlags: " << static_cast<unsigned>( header.packetFlags ) << std::endl;
os << space( indent ) << "packetLogicalLengthMinus1: " << header.packetLogicalLengthMinus1
<< std::endl;
os << space( indent ) << "packetLogicalLengthMinus1: " << packetLogicalLengthMinus1 << std::endl;
os << space( indent ) << "entryCount: " << entryCount << std::endl;
os << space( indent ) << "indexLevel: " << indexLevel << std::endl;
os << space( indent ) << "entryCount: " << header.entryCount << std::endl;
os << space( indent ) << "indexLevel: " << header.indexLevel << std::endl;
unsigned i;
for ( i = 0; i < entryCount && i < 10; i++ )
for ( i = 0; i < header.entryCount && i < 10; i++ )
{
os << space( indent ) << "entry[" << i << "]:" << std::endl;
os << space( indent + 4 ) << "chunkRecordNumber: " << entries[i].chunkRecordNumber
<< std::endl;
os << space( indent + 4 ) << "chunkPhysicalOffset: " << entries[i].chunkPhysicalOffset
<< std::endl;
}
if ( i < entryCount )
if ( i < header.entryCount )
{
os << space( indent ) << entryCount - i << "more entries unprinted..." << std::endl;
os << space( indent ) << header.entryCount - i << "more entries unprinted..." << std::endl;
}
}
#endif
Expand Down

0 comments on commit 6330aa1

Please sign in to comment.