Skip to content

Commit

Permalink
Port fix for CVE-2024-24786 (#152)
Browse files Browse the repository at this point in the history
From the official commit:

> encoding/protojson, internal/encoding/json: handle missing object values
>
> In internal/encoding/json, report an error when encountering a }
> when we are expecting an object field value. For example, the input
> `{"":}` now correctly results in an error at the closing } token.
>
> In encoding/protojson, check for an unexpected EOF token in
> skipJSONValue. This is redundant with the check in internal/encoding/json,
> but adds a bit more defense against any other similar bugs that
> might exist.
>
> Fixes CVE-2024-24786
>
> Change-Id: I03d52512acb5091c8549e31ca74541d57e56c99d
> Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/569356
> TryBot-Bypass: Damien Neil <dneil@google.com>
> Reviewed-by: Roland Shoemaker <roland@golang.org>
> Commit-Queue: Damien Neil <dneil@google.com>
  • Loading branch information
tdeebswihart committed Mar 11, 2024
1 parent 50a1089 commit 332690c
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 1 deletion.
2 changes: 1 addition & 1 deletion internal/protojson/json/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (d *Decoder) Read() (Token, error) {

case ObjectClose:
if len(d.openStack) == 0 ||
d.lastToken.kind == comma ||
d.lastToken.kind&(Name|comma) != 0 ||
d.openStack[len(d.openStack)-1] != ObjectOpen {
return Token{}, d.newSyntaxError(tok.pos, unexpectedFmt, tok.RawString())
}
Expand Down
15 changes: 15 additions & 0 deletions internal/protojson/json/decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1191,6 +1191,21 @@ func TestDecoder(t *testing.T) {
{E: errEOF},
},
},
{
in: `{""`,
want: []R{
{V: ObjectOpen},
{E: errEOF},
},
},
{
in: `{"":`,
want: []R{
{V: ObjectOpen},
{V: Name{""}},
{E: errEOF},
},
},
{
in: `{"34":"89",}`,
want: []R{
Expand Down
17 changes: 17 additions & 0 deletions internal/protojson/json_unmarshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2503,6 +2503,23 @@ func TestUnmarshal(t *testing.T) {
inputText: `{"foo":{"bar":[{"baz":[{}]]}}`,
umo: protojson.UnmarshalOptions{RecursionLimit: 5, DiscardUnknown: true},
wantErr: "exceeded max recursion depth",
}, {
desc: "Object missing value: no DiscardUnknown",
inputMessage: &testpb.TestAllTypes{},
inputText: `{"":}`,
umo: protojson.UnmarshalOptions{RecursionLimit: 5, DiscardUnknown: false},
wantErr: `(line 1:2): unknown field ""`,
}, {
desc: "Object missing value: DiscardUnknown",
inputMessage: &testpb.TestAllTypes{},
inputText: `{"":}`,
umo: protojson.UnmarshalOptions{RecursionLimit: 5, DiscardUnknown: true},
wantErr: `(line 1:5): unexpected token`,
}, {
desc: "Object missing value: Any",
inputMessage: &anypb.Any{},
inputText: `{"":}`,
wantErr: `(line 1:5): unexpected token`,
}}

for _, tt := range tests {
Expand Down
4 changes: 4 additions & 0 deletions internal/protojson/well_known_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,10 @@ func (d decoder) skipJSONValue() error {
if open > d.opts.RecursionLimit {
return errors.New("exceeded max recursion depth")
}
case json.EOF:
// This can only happen if there's a bug in Decoder.Read.
// Avoid an infinite loop if this does happen.
return errors.New("unexpected EOF")
}
if open == 0 {
return nil
Expand Down

0 comments on commit 332690c

Please sign in to comment.