unknownproto: check result from protowire.ConsumeFieldValue and return an error (#7770)

* unknownproto: check result from protowire.ConsumeFieldValue and return error

Given that protowire.ConsumeFieldValue returns -1 when it encounters an
error, perform a check for n < 0 and return the respectively obtained
error with context about the details.

Fixes an issue identified from a go-fuzz session, thanks to Ethan
Buchman and the IBC auditors from Informal Systems et al.

Fixes #7739.

* Address AlexanderBez's suggestions

* Use require in tests

* Add issue #7846 to TODO

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This commit is contained in:
Emmanuel T Odeke 2020-11-09 09:58:13 -08:00 committed by GitHub
parent 55772aec8c
commit fe8a891f11
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 0 deletions

View File

@ -0,0 +1,25 @@
package unknownproto_test
import (
"encoding/hex"
"io"
"testing"
"github.com/stretchr/testify/require"
"github.com/cosmos/cosmos-sdk/simapp"
)
// Issue #7739: Catch parse errors resulting from unexpected EOF in
// protowire.ConsumeFieldValue. Discovered from fuzzing.
func TestBadBytesPassedIntoDecoder(t *testing.T) {
data, _ := hex.DecodeString("0A9F010A9C200A2D2F6962632E636F72652E636F6E6E656374696F6E2E76312E4D7367436F6E6E656374696F584F75656E496E6974126B0A0D6962637A65726F636C69656E74120B6962637A65726F636F6E6E1A1C0A0C6962636F6E65636C69656E74120A6962636F6E65636F6E6E00002205312E302E302A283235454635364341373935313335453430393336384536444238313130463232413442453035433212080A0612040A0208011A40143342993E25DA936CDDC7BE3D8F603CA6E9661518D536D0C482E18A0154AA096E438A6B9BCADFCFC2F0D689DCCAF55B96399D67A8361B70F5DA13091E2F929")
cfg := simapp.MakeTestEncodingConfig()
decoder := cfg.TxConfig.TxDecoder()
tx, err := decoder(data)
// TODO: When issue https://github.com/cosmos/cosmos-sdk/issues/7846
// is addressed, we'll remove this .Contains check.
require.Contains(t, err.Error(), io.ErrUnexpectedEOF.Error())
require.Nil(t, tx)
}

View File

@ -92,6 +92,11 @@ func RejectUnknownFields(bz []byte, msg proto.Message, allowUnknownNonCriticals
// Skip over the bytes that store fieldNumber and wireType bytes.
bz = bz[m:]
n := protowire.ConsumeFieldValue(tagNum, wireType, bz)
if n < 0 {
err = fmt.Errorf("could not consume field value for tagNum: %d, wireType: %q; %w",
tagNum, wireTypeToString(wireType), protowire.ParseError(n))
return hasUnknownNonCriticals, err
}
fieldBytes := bz[:n]
bz = bz[n:]