Fix invalid free() with oneof (#647)

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.

1 files changed, 8 insertions, 1 deletions

diff --git a/pb_decode.c b/pb_decode.c
index 6469afb..4efe8a3 100644
--- a/pb_decode.c
+++ b/pb_decode.c
@@ -1150,7 +1150,14 @@ static bool pb_release_union_field(pb_istream_t *stream, pb_field_iter_t *iter)
      * This shouldn't fail unless the pb_field_t structure is corrupted. */
     if (!pb_field_iter_find(iter, new_tag))
         PB_RETURN_ERROR(stream, "iterator error");
-    
+
+    if (PB_ATYPE(iter->pos->type) == PB_ATYPE_POINTER)
+    {
+        /* Initialize the pointer to NULL to make sure it is valid
+         * even in case of error return. */
+        *(void**)iter->pData = NULL;
+    }
+
     return true;
 }