fix: ethtypes: handle length overflow case

[arajasek]

Related Issues

Similar to #11053

Proposed Changes

The string length field can overflow and turn negative here, leading to a panic

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

• Commits have a clear commit message.
• PR title is in the form of of <PR type>: <area>: <change being made>
  • example: fix: mempool: Introduce a cache for valid signatures
  • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
  • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
• New features have usage guidelines and / or documentation updates in
  • Lotus Documentation
  • Discussion Tutorials
• Tests exist for new functionality or change in behavior
• CI is green

[Stebalien]

removeme

[fridrik01]

You may want to do this check inside decodeLength (when checking lenInBytes+int(decodedLength) > len(data)) as that is overflowing the int64 to a negative number which is the root cause. That will also cover the case above when we are parsing lists which should have the same problem.

[arajasek]

@fridrik01 Isn't that the check you already added in #11079? Sorry, I'm a little confused.

In this PR I'm trying to catch an edge-case where decodedLength doesn't overflow, but decodedLength + strLenInBytes + 1 does.

[fridrik01]

@arajasek Yes I added a check for negative decodedLength (see L160 below) but we can add a check for overflow in L163:

lotus/chain/types/ethtypes/rlp.go

Lines 157 to 165 in 871d1ba

	if err := binary.Read(r, binary.BigEndian, &decodedLength); err != nil {
	return 0, xerrors.Errorf("invalid rlp data: cannot parse string length: %w", err)
	}
	if decodedLength < 0 {
	return 0, xerrors.Errorf("invalid rlp data: negative string length")
	}
	if lenInBytes+int(decodedLength) > len(data) {
	return 0, xerrors.Errorf("invalid rlp data: out of bound while parsing list")
	}

So something like

totalLen := lenInBytes+decodedLength
if totalLen < 0 || totalLen > len(data) {
    return 0, xerrors.Errorf("invalid rlp data: out of bound while parsing list")
}

Whats happening is that decodedLength is close to Int64 max so when we add lenInBytes it overflows and becomes negative.

[arajasek]

Ah, got it! I think I'm gonna do both -- your suggestion SGTM, but we do have to check again in the caller because we wind up indexing data[totalLen + 1]