C#: Wrong deserialization of Int32Value (from wrappers.proto) for negative values when segment boundary is crossed

[PeterMaco]

What version of protobuf and what language are you using?
Version: v3.13.0
Language: C#

What operating system (Linux, Windows, ...) and version?
Reproducible on any version

What runtime / compiler are you using (e.g., python version or gcc version)
.NET Core

What did you do?
There is a problem with the deserialization of Int32 field when it contains a negative value. This happens only on very specific circumstances - when the field is serialized across the buffer (span) boundaries (when it starts at the and of one span and continues at the beginning of the next span). Deserialization thinks that the whole field fits to the first span, but this is not true and after deserialization, the buffer pointer is beyond the buffer size.

What did you expect to see
Successful deserialization of the message on the client

What did you see instead?
Exception:
System.IndexOutOfRangeException: Index was outside the bounds of the array.
at Google.Protobuf.ParsingPrimitives.ReadRawByte(ReadOnlySpan1& buffer, ParserInternalState& state) at Google.Protobuf.ParsingPrimitives.ParseRawVarint32SlowPath(ReadOnlySpan1& buffer, ParserInternalState& state)
at Google.Protobuf.ParsingPrimitivesWrappers.ReadUInt32WrapperSlow(ReadOnlySpan1& buffer, ParserInternalState& state) at Google.Protobuf.ParsingPrimitivesWrappers.ReadInt32Wrapper(ReadOnlySpan1& buffer, ParserInternalState& state)

Details
This is the code for int32 serialization. Negative values are serialized as Varint64, it can be 11 bytes long.

public static void WriteInt32(ref Span buffer, ref WriterInternalState state, int value)
{
if (value >= 0)
{
WriteRawVarint32(ref buffer, ref state, (uint)value);
}
else
{
// Must sign-extend.
WriteRawVarint64(ref buffer, ref state, (ulong)value);
}
}

But deserialization do not count with this. ReadUInt32Wrapper function has this condition there:

// length:1 + tag:1 + value:5(varint32-max) = 7 bytes
if (state.bufferPos + 7 <= state.bufferSize)
{
// The entire wrapper message is already contained in buffer.

... which is not true for negative values.

[jtattermusch]

Can you provide an example of byte sequence that is parsed incorrectly?

[jtattermusch]

CC @JamesNK

[PeterMaco]

Value -1 encoded:

202
1
11
8
254
255
255
255
255
255
255
255
255
1

[JamesNK]

Fix - #8035

[jtattermusch]

FTR quoting from the official docs:

If you use int32 or int64 as the type for a negative number, the resulting varint is always ten bytes long – it is, effectively, treated like a very large unsigned integer.
https://developers.google.com/protocol-buffers/docs/encoding#types

[jtattermusch]

Note that this bug only affects parsing of the Int32Value type from wrappers.proto:

protobuf/src/google/protobuf/wrappers.proto

Line 88 in 397d34c

message Int32Value {