From afdf6dafc224258c450b47c5c7fcbc9365bbc302 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 10 Jan 2023 15:18:25 -0800 Subject: [PATCH] Fix proto deserialization issue when parsing a packed repeated enum field whose possible values form a contiguous range that starts with 0 or 1, and end no greater than 126, when parsing a noncontiguous stream (such as from a Cord). This routine was misusing `ParseContext::Done`. It should only be used at tag boundaries and not within field parsing. PiperOrigin-RevId: 501107542 --- .../generated_message_tctable_lite.cc | 45 ++++--------------- src/google/protobuf/parse_context.h | 2 + 2 files changed, 10 insertions(+), 37 deletions(-) diff --git a/src/google/protobuf/generated_message_tctable_lite.cc b/src/google/protobuf/generated_message_tctable_lite.cc index cd70aeb3515f..b6abc182d2da 100644 --- a/src/google/protobuf/generated_message_tctable_lite.cc +++ b/src/google/protobuf/generated_message_tctable_lite.cc @@ -1316,52 +1316,23 @@ const char* TcParser::PackedEnumSmallRange(PROTOBUF_TC_PARAM_DECL) { PROTOBUF_MUSTTAIL return MiniParse(PROTOBUF_TC_PARAM_PASS); } } + + // Since ctx->ReadPackedVarint does not use TailCall or Return, sync any + // pending hasbits now: + SyncHasbits(msg, hasbits, table); + const auto saved_tag = UnalignedLoad(ptr); ptr += sizeof(TagType); auto* field = &RefAt>(msg, data.offset()); const uint8_t max = data.aux_idx(); - int size = static_cast(ReadSize(&ptr)); - if (PROTOBUF_PREDICT_FALSE(ptr == nullptr)) { - return Error(PROTOBUF_TC_PARAM_PASS); - } - - while (size > 0) { - uint8_t v = *ptr; + return ctx->ReadPackedVarint(ptr, [=](int32_t v) { if (PROTOBUF_PREDICT_FALSE(min > v || v > max)) { - // If it is out of range it might be because it is a non-canonical varint - // with a small value (ie the continuation bit is on). - // Do the full varint parse just in case. - uint64_t tmp; - const char* const old_ptr = ptr; - ptr = ParseVarint(ptr, &tmp); - if (PROTOBUF_PREDICT_FALSE(ptr == nullptr)) { - return Error(PROTOBUF_TC_PARAM_PASS); - } - size -= static_cast(ptr - old_ptr); - if (PROTOBUF_PREDICT_FALSE(size < 0)) { - return Error(PROTOBUF_TC_PARAM_PASS); - } - int32_t v32 = static_cast(tmp); - if (PROTOBUF_PREDICT_FALSE(min > v32 || v32 > max)) { - UnknownPackedEnum(msg, table, FastDecodeTag(saved_tag), v32); - } else { - field->Add(v32); - } + UnknownPackedEnum(msg, table, FastDecodeTag(saved_tag), v); } else { - ptr += 1; - size -= 1; field->Add(v); } - - if (PROTOBUF_PREDICT_FALSE(!ctx->DataAvailable(ptr))) { - if (ctx->Done(&ptr) && size != 0) { - return Error(PROTOBUF_TC_PARAM_PASS); - } - } - } - - PROTOBUF_MUSTTAIL return ToParseLoop(PROTOBUF_TC_PARAM_PASS); + }); } PROTOBUF_NOINLINE const char* TcParser::FastEr0P1(PROTOBUF_TC_PARAM_DECL) { diff --git a/src/google/protobuf/parse_context.h b/src/google/protobuf/parse_context.h index 2b91807d5fdd..8e2c6b060046 100644 --- a/src/google/protobuf/parse_context.h +++ b/src/google/protobuf/parse_context.h @@ -413,6 +413,8 @@ class PROTOBUF_EXPORT ParseContext : public EpsCopyInputStream { void TrackCorrectEnding() { group_depth_ = 0; } + // Done should only be called when the parsing pointer is pointing to the + // beginning of field data - that is, at a tag. Or if it is NULL. bool Done(const char** ptr) { return DoneWithCheck(ptr, group_depth_); } int depth() const { return depth_; }