Do not assume that Python has already been found by CMake. Fix value of
CMake variable PYTHON_EXECUTABLE if Python 3 was found. Change minimum
supported Python version to 2.6.
This fixes a bug introduced by this commit:
d8d3b75e2e
1) Search explicitly for python2.7
In systems where python3 is default or in build cases where the user has
already searched for and found python3 in CMake, store the python3
executable and search for python2.7.
2) Generate nanopb core protobuf files with CMake
Generate python output files used in turn by the nanopb generator
script. This removes the requirement of manually calling 'make' in the
nanopb/generator/proto directory.
3) Use nanopb options file if it exists
Look for nanopb options file and use in protobuf source and header
generation if it exists. The options file must have the same name and
path as the proto file, excluding the extension.
The sizes are represented as EncodedSize() instances, which cause
max() operation to sort them by address instead of value. This caused
pretty much random item to be selected for the maximum.
Update issue 141
Status: FixedInGit
There was a memory leak when:
1) A statically allocated submessage or
2) an extension field submessage
contained
A) a pointer-type field or
B) a submessage that further contained a pointer-type field.
This was because pb_release() didn't recurse into non-pointer fields.
Update issue 138
Status: FixedInGit
Possible consequences of bug:
1) Denial of service by causing a crash
Possible when all of the following apply:
- Untrusted data is passed to pb_decode()
- The top-level message contains a static string field as the first field.
Causes a single write of '0' byte to 1 byte before the message struct.
2) Remote code execution
Possible when all of the following apply:
- 64-bit platform
- The message or a submessage contains a static/pointer string field.
- Decoding directly from a custom pb_istream_t
- bytes_left on the stream is set to larger than 4 GB
Causes a write of up to 4 GB of data past the string field.
3) Possible heap corruption or remote code execution
Possible when all of the following apply:
- less than 64-bit platform
- The message or a submessage contains a pointer-type bytes field.
Causes a write of sizeof(pb_size_t) bytes of data past a 0-byte long
malloc()ed buffer. On many malloc() implementations, this causes at
most a crash. However, remote code execution through a controlled jump
cannot be ruled out.
--
Detailed analysis follows
In the following consideration, I define "platform bitness" as equal to
number of bits in size_t datatype. Therefore most 8-bit platforms are
regarded as 16-bit for the purposes of this discussion.
1. The overflow in pb_dec_string
The overflow happens in this computation:
uint32_t size;
size_t alloc_size;
alloc_size = size + 1;
There are two ways in which the overflow can occur: In the uint32_t
addition, or in the cast to size_t. This depends on the platform
bitness.
On 32- and 64-bit platforms, the size has to be UINT32_MAX for the
overflow to occur. In that case alloc_size will be 0.
On 16-bit platforms, overflow will happen whenever size is more than
UINT16_MAX, and resulting alloc_size is attacker controlled.
For static fields, the alloc_size value is just checked against the
field data size. For pointer fields, the alloc_size value is passed to
malloc(). End result in both cases is the same, the storage is 0 or
just a few bytes in length.
On 16-bit platforms, another overflow occurs in the call to pb_read(),
when passing the original size. An attacker will want the passed value
to be larger than the alloc_size, therefore the only reasonable choice
is to have size = UINT16_MAX and alloc_size = 0. Any larger multiple
will truncate to the same values.
At this point we have read atleast the tag and the string length of the
message, i.e. atleast 3 bytes. The maximum initial value for stream
bytes_left is SIZE_MAX, thus at this point at most SIZE_MAX-3 bytes are
remaining.
On 32-bit and 16-bit platforms this means that the size passed to
pb_read() is always larger than the number of remaining bytes. This
causes pb_read() to fail immediately, before reading any bytes.
On 64-bit platforms, it is possible for the bytes_left value to be set
to a value larger than UINT32_MAX, which is the wraparound point in
size calculation. In this case pb_read() will succeed and write up to 4
GB of attacker controlled data over the RAM that comes after the string
field.
On all platforms, there is an unconditional write of a terminating null
byte. Because the size of size_t typically reflects the size of the
processor address space, a write at UINT16_MAX or UINT32_MAX bytes
after the string field actually wraps back to before the string field.
Consequently, on 32-bit and 16-bit platforms, the bug causes a single
write of '0' byte at one byte before the string field.
If the string field is in the middle of a message, this will just
corrupt other data in the message struct. Because the message contents
is attacker controlled anyway, this is a non-issue. However, if the
string field is the first field in the top-level message, it can
corrupt other data on the stack/heap before it. Typically a single '0'
write at a location not controlled by attacker is enough only for a
denial-of-service attack.
When using pointer fields and malloc(), the attacker controlled
alloc_size will cause a 0-size allocation to happen. By the same logic
as before, on 32-bit and 16-bit platforms this causes a '0' byte write
only. On 64-bit platforms, however, it will again allow up to 4 GB of
malicious data to be written over memory, if the stream length allows
the read.
2. The overflow in pb_dec_bytes
This overflow happens in the PB_BYTES_ARRAY_T_ALLOCSIZE macro:
The computation is done in size_t data type this time. This means that
an overflow is possible only when n is larger than SIZE_MAX -
offsetof(..). The offsetof value in this case is equal to
sizeof(pb_size_t) bytes.
Because the incoming size value is limited to 32 bits, no overflow can
happen here on 64-bit platforms.
The size will be passed to pb_read(). Like before, on 32-bit and 16-bit
platforms the read will always fail before writing anything.
This leaves only the write of bdest->size as exploitable. On statically
allocated fields, the size field will always be allocated, regardless
of alloc_size. In this case, no buffer overflow is possible here, but
user code could possibly use the attacker controlled size value and
read past a buffer.
If the field is allocated through malloc(), this will allow a write of
sizeof(pb_size_t) attacker controlled bytes to past a 0-byte long
buffer. In typical malloc implementations, this will either fit in
unused alignment padding area, or cause a heap corruption and a crash.
Under very exceptional situation it could allow attacker to influence
the behaviour of malloc(), possibly jumping into an attacker-controlled
location and thus leading to remote code execution.
If a required or optional field appeared twice in the message data,
pb_decode will overwrite the old data with new one. That is fine, but
with submessage fields, it didn't release the allocated subfields before
overwriting.
This bug can manifest if all of the following conditions are true:
1. There is a message with a "optional" or "required" submessage field
that has type:FT_POINTER.
2. The submessage contains atleast one field with type:FT_POINTER.
3. The message data to be decoded has the submessage field twice in it.
There was a double-free bug in pb_release() because it didn't set size fields
to zero after deallocation. Most commonly this happens if pb_decode() fails,
internally calls pb_release() and then application code also calls pb_release().
