Skip to content

Commit e9bfb8f

Browse files
committed
Add varsize out of bounds checks
1 parent 4a48d77 commit e9bfb8f

7 files changed

Lines changed: 411 additions & 35 deletions

File tree

runtime/ClangTidySuppressions.txt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ cppcoreguidelines-pro-bounds-array-to-pointer-decay:src/zserio/ZserioTreeCreator
7474

7575
# The following is filtered out because bounds are checked naturally by implementation. Therefore method 'at'
7676
# would only bring the performance drop.
77-
cppcoreguidelines-pro-bounds-constant-array-index:src/zserio/BitStreamReader.cpp:301
78-
cppcoreguidelines-pro-bounds-constant-array-index:src/zserio/BitStreamReader.cpp:315
77+
cppcoreguidelines-pro-bounds-constant-array-index:src/zserio/BitStreamReader.cpp:303
78+
cppcoreguidelines-pro-bounds-constant-array-index:src/zserio/BitStreamReader.cpp:317
7979
cppcoreguidelines-pro-bounds-constant-array-index:src/zserio/BitStreamWriter.cpp:351
8080
cppcoreguidelines-pro-bounds-constant-array-index:src/zserio/BitStreamWriter.cpp:362
8181
cppcoreguidelines-pro-bounds-constant-array-index:src/zserio/BitStreamWriter.cpp:373
@@ -114,6 +114,10 @@ cppcoreguidelines-pro-type-reinterpret-cast:src/zserio/ValidationSqliteUtil.h:10
114114
fuchsia-multiple-inheritance:src/zserio/IntrospectableView.h:881
115115
fuchsia-multiple-inheritance:src/zserio/ReflectableData.h:516
116116

117+
# Strong type class wrapper should use implicit conversion
118+
google-explicit-constructor:src/zserio/BitStreamReader.h:22
119+
google-explicit-constructor:src/zserio/BitStreamReader.h:25
120+
117121
# Optional follows std::optional so the ctors should be implicit.
118122
google-explicit-constructor:src/zserio/Optional.h:127
119123
google-explicit-constructor:src/zserio/Optional.h:137

runtime/src/zserio/BitStreamReader.cpp

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#include "zserio/FloatUtil.h"
77
#include "zserio/RuntimeArch.h"
88

9+
const size_t ZSERIO_MAX_INITIAL_ARRAY_ALLOCATION = 128 * 1024 * 1024;
10+
911
namespace zserio
1012
{
1113

@@ -355,11 +357,13 @@ inline uint64_t readUnsignedBits64Impl(ReaderContext& ctx, uint8_t numBits)
355357

356358
} // namespace
357359

358-
BitStreamReader::ReaderContext::ReaderContext(Span<const uint8_t> readBuffer, size_t readBufferBitSize) :
360+
BitStreamReader::ReaderContext::ReaderContext(
361+
Span<const uint8_t> readBuffer, size_t readBufferBitSize, size_t maxArrayPrealloc) :
359362
buffer(readBuffer),
360363
bufferBitSize(readBufferBitSize),
361364
cache(0),
362365
cacheNumBits(0),
366+
maxArrayPreallocation(maxArrayPrealloc),
363367
bitIndex(0)
364368
{
365369
if (buffer.size() > MAX_BUFFER_SIZE)
@@ -369,16 +373,18 @@ BitStreamReader::ReaderContext::ReaderContext(Span<const uint8_t> readBuffer, si
369373
}
370374
}
371375

372-
BitStreamReader::BitStreamReader(const uint8_t* buffer, size_t bufferByteSize) :
373-
BitStreamReader(Span<const uint8_t>(buffer, bufferByteSize))
376+
BitStreamReader::BitStreamReader(
377+
const uint8_t* buffer, size_t bufferByteSize, ArrayAllocation maxArrayPrealloc) :
378+
BitStreamReader(Span<const uint8_t>(buffer, bufferByteSize), maxArrayPrealloc)
374379
{}
375380

376-
BitStreamReader::BitStreamReader(Span<const uint8_t> buffer) :
377-
m_context(buffer, buffer.size() * 8)
381+
BitStreamReader::BitStreamReader(Span<const uint8_t> buffer, ArrayAllocation maxArrayPrealloc) :
382+
m_context(buffer, buffer.size() * 8, maxArrayPrealloc)
378383
{}
379384

380-
BitStreamReader::BitStreamReader(Span<const uint8_t> buffer, size_t bufferBitSize) :
381-
m_context(buffer, bufferBitSize)
385+
BitStreamReader::BitStreamReader(
386+
Span<const uint8_t> buffer, size_t bufferBitSize, ArrayAllocation maxArrayPrealloc) :
387+
m_context(buffer, bufferBitSize, maxArrayPrealloc)
382388
{
383389
if (buffer.size() < (bufferBitSize + 7) / 8)
384390
{
@@ -387,8 +393,9 @@ BitStreamReader::BitStreamReader(Span<const uint8_t> buffer, size_t bufferBitSiz
387393
}
388394
}
389395

390-
BitStreamReader::BitStreamReader(const uint8_t* buffer, size_t bufferBitSize, BitsTag) :
391-
m_context(Span<const uint8_t>(buffer, (bufferBitSize + 7) / 8), bufferBitSize)
396+
BitStreamReader::BitStreamReader(
397+
const uint8_t* buffer, size_t bufferBitSize, BitsTag, ArrayAllocation maxArrayPrealloc) :
398+
m_context(Span<const uint8_t>(buffer, (bufferBitSize + 7) / 8), bufferBitSize, maxArrayPrealloc)
392399
{}
393400

394401
uint32_t BitStreamReader::readUnsignedBits32(uint8_t numBits)

runtime/src/zserio/BitStreamReader.h

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,26 @@
1111
#include "zserio/String.h"
1212
#include "zserio/Types.h"
1313

14+
extern const size_t ZSERIO_MAX_INITIAL_ARRAY_ALLOCATION;
15+
1416
namespace zserio
1517
{
1618

19+
class ArrayAllocation
20+
{
21+
public:
22+
ArrayAllocation(size_t alloc = ZSERIO_MAX_INITIAL_ARRAY_ALLOCATION) :
23+
allocation(alloc)
24+
{}
25+
operator size_t() const
26+
{
27+
return allocation;
28+
}
29+
30+
private:
31+
size_t allocation;
32+
};
33+
1734
/**
1835
* Reader class which allows to read various data from the bit stream.
1936
*/
@@ -34,7 +51,8 @@ class BitStreamReader
3451
* \param readBuffer Span to the buffer to read.
3552
* \param readBufferBitSize Size of the buffer in bits.
3653
*/
37-
explicit ReaderContext(Span<const uint8_t> readBuffer, size_t readBufferBitSize);
54+
explicit ReaderContext(
55+
Span<const uint8_t> readBuffer, size_t readBufferBitSize, size_t maxArrayPrealloc);
3856

3957
/**
4058
* Destructor.
@@ -59,6 +77,7 @@ class BitStreamReader
5977

6078
uintptr_t cache; /**< Bit cache to optimize bit reading. */
6179
uint8_t cacheNumBits; /**< Num bits available in the bit cache. */
80+
const size_t maxArrayPreallocation; /**< Maximum initial array allocation. */
6281

6382
BitPosType bitIndex; /**< Current bit index. */
6483
};
@@ -69,39 +88,42 @@ class BitStreamReader
6988
* \param buffer Pointer to the buffer to read.
7089
* \param bufferByteSize Size of the buffer in bytes.
7190
*/
72-
explicit BitStreamReader(const uint8_t* buffer, size_t bufferByteSize);
91+
explicit BitStreamReader(
92+
const uint8_t* buffer, size_t bufferByteSize, ArrayAllocation maxArrayPrealloc = {});
7393

7494
/**
7595
* Constructor from buffer passed as a Span.
7696
*
7797
* \param buffer Buffer to read.
7898
*/
79-
explicit BitStreamReader(Span<const uint8_t> buffer);
99+
explicit BitStreamReader(Span<const uint8_t> buffer, ArrayAllocation maxArrayPrealloc = {});
80100

81101
/**
82102
* Constructor from buffer passed as a Span with exact bit size.
83103
*
84104
* \param buffer Buffer to read.
85105
* \param bufferBitSize Size of the buffer in bits.
86106
*/
87-
explicit BitStreamReader(Span<const uint8_t> buffer, size_t bufferBitSize);
107+
explicit BitStreamReader(
108+
Span<const uint8_t> buffer, size_t bufferBitSize, ArrayAllocation maxArrayPrealloc = {});
88109

89110
/**
90111
* Constructor from raw buffer with exact bit size.
91112
*
92113
* \param buffer Pointer to buffer to read.
93114
* \param bufferBitSize Size of the buffer in bits.
94115
*/
95-
explicit BitStreamReader(const uint8_t* buffer, size_t bufferBitSize, BitsTag);
116+
explicit BitStreamReader(
117+
const uint8_t* buffer, size_t bufferBitSize, BitsTag, ArrayAllocation maxArrayPrealloc = {});
96118

97119
/**
98120
* Constructor from bit buffer.
99121
*
100122
* \param bitBuffer Bit buffer to read from.
101123
*/
102124
template <typename ALLOC>
103-
explicit BitStreamReader(const BasicBitBuffer<ALLOC>& bitBuffer) :
104-
BitStreamReader(bitBuffer.getData(), bitBuffer.getBitSize())
125+
explicit BitStreamReader(const BasicBitBuffer<ALLOC>& bitBuffer, ArrayAllocation maxArrayPrealloc = {}) :
126+
BitStreamReader(bitBuffer.getData(), bitBuffer.getBitSize(), maxArrayPrealloc)
105127
{}
106128

107129
/**
@@ -248,6 +270,10 @@ class BitStreamReader
248270
{
249271
const size_t len = static_cast<size_t>(readVarSize());
250272
const BitPosType beginBitPosition = getBitPosition();
273+
if (beginBitPosition + 8 * len > getBufferBitSize())
274+
{
275+
throw CppRuntimeException("BitStreamReader: byte array size exceeds available buffer!");
276+
}
251277
if ((beginBitPosition & 0x07U) != 0)
252278
{
253279
// we are not aligned to byte
@@ -280,6 +306,10 @@ class BitStreamReader
280306
{
281307
const size_t len = static_cast<size_t>(readVarSize());
282308
const BitPosType beginBitPosition = getBitPosition();
309+
if (beginBitPosition + 8 * len > getBufferBitSize())
310+
{
311+
throw CppRuntimeException("BitStreamReader: byte array size exceeds available buffer!");
312+
}
283313
if ((beginBitPosition & 0x07U) != 0)
284314
{
285315
// we are not aligned to byte
@@ -313,11 +343,15 @@ class BitStreamReader
313343
BasicBitBuffer<RebindAlloc<ALLOC, uint8_t>> readBitBuffer(const ALLOC& allocator = ALLOC())
314344
{
315345
const size_t bitSize = static_cast<size_t>(readVarSize());
346+
const BitPosType beginBitPosition = getBitPosition();
347+
if (beginBitPosition + bitSize > getBufferBitSize())
348+
{
349+
throw CppRuntimeException("BitStreamReader: byte array size exceeds available buffer!");
350+
}
316351
const size_t numBytesToRead = bitSize / 8;
317352
const uint8_t numRestBits = static_cast<uint8_t>(bitSize - numBytesToRead * 8);
318353
BasicBitBuffer<RebindAlloc<ALLOC, uint8_t>> bitBuffer(bitSize, allocator);
319354
Span<uint8_t> buffer = bitBuffer.getData();
320-
const BitPosType beginBitPosition = getBitPosition();
321355
const Span<uint8_t>::iterator itEnd = buffer.begin() + numBytesToRead;
322356
if ((beginBitPosition & 0x07U) != 0)
323357
{
@@ -377,6 +411,16 @@ class BitStreamReader
377411
return m_context.bufferBitSize;
378412
}
379413

414+
/**
415+
* Gets the maximum initial array capacity in bytes.
416+
*
417+
* \return max initial array capacity
418+
*/
419+
size_t getMaxArrayPreallocation() const
420+
{
421+
return m_context.maxArrayPreallocation;
422+
}
423+
380424
private:
381425
uint8_t readByte();
382426

0 commit comments

Comments
 (0)