From 5ef128616baffd15bb904fc56f651d40901be429 Mon Sep 17 00:00:00 2001 From: Petteri Aimonen Date: Sat, 17 May 2014 20:06:55 +0300 Subject: Fix security issue with PB_ENABLE_MALLOC. The multiplication in allocate_field could potentially overflow, leading to allocating too little memory. This could subsequently allow an attacker to cause a write past the buffer, overwriting other memory contents. The attack is possible if untrusted message data is decoded using nanopb, and the message type includes a pointer-type string or bytes field, or a repeated numeric field. Submessage fields are not affected. This issue only affects systems that have been compiled with PB_ENABLE_MALLOC enabled. Only version nanopb-0.2.7 is affected, as prior versions do not include this functionality. Update issue 117 Status: FixedInGit --- pb_decode.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/pb_decode.c b/pb_decode.c index 9a48c60f..d687cee3 100644 --- a/pb_decode.c +++ b/pb_decode.c @@ -470,11 +470,31 @@ static bool checkreturn decode_static_field(pb_istream_t *stream, pb_wire_type_t #ifdef PB_ENABLE_MALLOC /* Allocate storage for the field and store the pointer at iter->pData. - * array_size is the number of entries to reserve in an array. */ + * array_size is the number of entries to reserve in an array. + */ static bool checkreturn allocate_field(pb_istream_t *stream, void *pData, size_t data_size, size_t array_size) { void *ptr = *(void**)pData; - size_t size = array_size * data_size; + + /* Check for multiplication overflows. */ + size_t size = 0; + if (data_size > 0 && array_size > 0) + { + /* Avoid the costly division if the sizes are small enough. + * Multiplication is safe as long as only half of bits are set + * in either multiplicand. + */ + const size_t check_limit = (size_t)1 << (sizeof(size_t) * 4); + if (data_size >= check_limit || array_size >= check_limit) + { + if (SIZE_MAX / array_size < data_size) + { + PB_RETURN_ERROR(stream, "size too large"); + } + } + + size = array_size * data_size; + } /* Allocate new or expand previous allocation */ /* Note: on failure the old pointer will remain in the structure, -- cgit 1.2.3-korg