From 31e09c6630c47d0aa1c1ec3909a936ba5674254f Mon Sep 17 00:00:00 2001 From: Tobba Date: Wed, 21 Dec 2016 21:02:53 +0100 Subject: Fix closing a non-empty substream resulting in an incorrect stream state --- docs/migration.rst | 13 +++++++++++++ pb_decode.c | 29 +++++++++++++++++++++-------- pb_decode.h | 2 +- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/docs/migration.rst b/docs/migration.rst index cd5911f..2d9ce38 100644 --- a/docs/migration.rst +++ b/docs/migration.rst @@ -11,6 +11,19 @@ are included, in order to make it easier to find this document. .. contents :: +Nanopb-0.3.8 (2017-xx-xx) +========================= +Fully drain substreams before closing + +**Rationale:** If the substream functions were called directly and the caller +did not completely empty the substring before closing it, the parent stream +would be put into an incorrect state. + +**Changes:** *pb_close_string_substream* can now error and returns a boolean. + +**Required actions:** Add error checking onto any call to +*pb_close_string_substream*. + Nanopb-0.3.5 (2016-02-13) ========================= diff --git a/pb_decode.c b/pb_decode.c index 1fcc4e5..92d0175 100644 --- a/pb_decode.c +++ b/pb_decode.c @@ -333,13 +333,19 @@ bool checkreturn pb_make_string_substream(pb_istream_t *stream, pb_istream_t *su return true; } -void pb_close_string_substream(pb_istream_t *stream, pb_istream_t *substream) +bool checkreturn pb_close_string_substream(pb_istream_t *stream, pb_istream_t *substream) { + if (substream->bytes_left) { + if (!pb_read(substream, NULL, substream->bytes_left)) + return false; + } + stream->state = substream->state; #ifndef PB_NO_ERRMSG stream->errmsg = substream->errmsg; #endif + return true; } /************************* @@ -385,11 +391,12 @@ static bool checkreturn decode_static_field(pb_istream_t *stream, pb_wire_type_t } (*size)++; } - pb_close_string_substream(stream, &substream); - + if (substream.bytes_left != 0) PB_RETURN_ERROR(stream, "array overflow"); - + if (!pb_close_string_substream(stream, &substream)) + return false; + return status; } else @@ -569,7 +576,8 @@ static bool checkreturn decode_pointer_field(pb_istream_t *stream, pb_wire_type_ (*size)++; } - pb_close_string_substream(stream, &substream); + if (!pb_close_string_substream(stream, &substream)) + return false; return status; } @@ -623,7 +631,9 @@ static bool checkreturn decode_callback_field(pb_istream_t *stream, pb_wire_type PB_RETURN_ERROR(stream, "callback failed"); } while (substream.bytes_left); - pb_close_string_substream(stream, &substream); + if (!pb_close_string_substream(stream, &substream)) + return false; + return true; } else @@ -964,7 +974,9 @@ bool pb_decode_delimited(pb_istream_t *stream, const pb_field_t fields[], void * return false; status = pb_decode(&substream, fields, dest_struct); - pb_close_string_substream(stream, &substream); + + if (!pb_close_string_substream(stream, &substream)) + return false; return status; } @@ -1343,6 +1355,7 @@ static bool checkreturn pb_dec_submessage(pb_istream_t *stream, const pb_field_t else status = pb_decode_noinit(&substream, submsg_fields, dest); - pb_close_string_substream(stream, &substream); + if (!pb_close_string_substream(stream, &substream)) + return false; return status; } diff --git a/pb_decode.h b/pb_decode.h index e7eb209..a426bdd 100644 --- a/pb_decode.h +++ b/pb_decode.h @@ -144,7 +144,7 @@ bool pb_decode_fixed64(pb_istream_t *stream, void *dest); /* Make a limited-length substream for reading a PB_WT_STRING field. */ bool pb_make_string_substream(pb_istream_t *stream, pb_istream_t *substream); -void pb_close_string_substream(pb_istream_t *stream, pb_istream_t *substream); +bool pb_close_string_substream(pb_istream_t *stream, pb_istream_t *substream); #ifdef __cplusplus } /* extern "C" */ -- cgit v1.2.3