Remove the `extra/poetry` directory to eliminate high-severity vulnerabilities
flagged in unused protobuf dependencies. Also remove the reference to it from
`tools/set_version.sh` and fix existing shellcheck warnings in the script.
Add the missing `permissions: contents: read` configuration to workflows
and pin the `actions/checkout` step to a specific commit hash for security
consistency with other Firebase repositories.
As far as I can tell, the logic above this line does work correctly and
calls `allocate_field()` in any case where iter->pData could point to pointer
to NULL. But the logic depends on PB_SIZE_MAX and other subtle points, which
may be why static analyzers complain. This commit makes it explicitly check
and error out.
Nanopb would call free() or realloc() on an invalid
(attacker controlled) pointer value when all the following
conditions are true:
- PB_ENABLE_MALLOC is defined at the compile time
- Message definition contains an oneof field, and the oneof
contains at least one pointer type field and at least one
non-pointer type field.
- Data being decoded first contains a non-pointer value for
the oneof field, and later contains an overwriting pointer
value.
Depending on message layout, the bug may not be exploitable in all
cases, but it is known to be exploitable at least with string and
bytes fields. Actual security impact will also depend on the heap
implementation used.
The 0.4 branch contains commit 2519119bab ("Verify stream size before
allocating string / bytes."):
This stops ridicuously large mallocs from getting through
on length-limited streams or buffers. Typically you should
also override realloc() to limit allocation size yourself
if dealing with untrusted data in pointer mode, but this
at least limits the potential denial-of-service attacks.
Backport this patch in 0.3 maintenance branch in order to make fuzzing
not crash as soon as the decoder tries to allocate a large buffer.
Nanopb would leak memory when all of the following conditions were true:
- PB_ENABLE_MALLOC is defined at the compile time
- Message definitions contains an oneof field,
the oneof contains a static submessage, and
the static submessage contains a pointer field.
- Data being decoded contains two values for the submessage.
The logic in pb_release_union_field would detect that the same
submessage occurs twice, and wouldn't release it because keeping
the old values is necessary to match the C++ library behavior
regarding message merges.
But then decode_static_field() would go to memset() the whole
submessage to zero, because it unconditionally assumed it to
be uninitialized memory. This would normally happen when the
contents of the union field is switched to a different oneof
item, instead of merging with the same one.
This commit changes it so that the field is memset() only when
`which_field` contains a different tag.
This also covers the fairly rarely used behavior of protobuf C++
library regarding oneof merges: if an oneof submessage occurs
multiple times in a message, their contents are merged together.
This behavior was also previously broken in nanopb.
This update allows clients to share common syntax for nanopb imports/includes between CocoaPods and Swift Package Manager.
See example at firebase/firebase-ios-sdk#6165
On platforms where size_t equals pb_size_t, for example AVR where both
are 16-bit, or x86 and ARM when PB_FIELD_32BIT is defined, the buffer size
checks in pb_write() and pb_enc_submessage can overflow if a bytes field
has size close to maximum size value. This causes read and write out of bounds.
This issue can cause a security vulnerability if the size of a bytes field
in the structure given to pb_encode() is untrusted. Note that pb_decode()
has correct bounds checking and will reject too large values.
Nick Cooke