Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: when decoding addresses restrict to 20 bytes #56

Merged
merged 2 commits into from
Jan 17, 2024
Merged

fix: when decoding addresses restrict to 20 bytes #56

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e47e379
fix: when decoding addresses restrict to 20 bytes
SamMayWork Jan 10, 2024
711fd0f
fix: read bytes corresponding to the suffix number
SamMayWork Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion pkg/abi/abidecode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright © 2023 Kaleido, Inc.
// Copyright © 2024 Kaleido, Inc.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down Expand Up @@ -131,6 +131,16 @@ func decodeABIUnsignedInt(ctx context.Context, desc string, block []byte, _, hea
return cv, err
}

func decodeABIAddress(ctx context.Context, desc string, block []byte, _, headPosition int, component *typeComponent) (cv *ComponentValue, err error) {
// Addresses are 20 bytes, but are represented as a 32 bit unsigned integer so we remove the proceeding 12 bytes and then read the rest as the address
cv = &ComponentValue{Component: component}
if headPosition+32 > len(block) {
return nil, i18n.NewError(ctx, signermsgs.MsgNotEnoughBytesABIValue, component, desc)
}
cv.Value = new(big.Int).SetBytes(block[headPosition+12 : headPosition+32])
return cv, err
}

func intToFixed(ctx context.Context, component *typeComponent, cv *ComponentValue) (*ComponentValue, error) {
if component.n == 0 {
return nil, i18n.NewError(ctx, signermsgs.MsgBadABITypeComponent, component)
Expand Down
25 changes: 24 additions & 1 deletion pkg/abi/abidecode_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright © 2022 Kaleido, Inc.
// Copyright © 2024 Kaleido, Inc.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down Expand Up @@ -1027,3 +1027,26 @@ func TestDecodeABIElementInsufficientDataTuple(t *testing.T) {
_, _, err = decodeABIElement(context.Background(), "", block, 0, 0, tc.(*typeComponent).tupleChildren[0])
assert.Regexp(t, "FF22045", err)
}

func TestDecodeAddressWithNonZeroPadding(t *testing.T) {

f := &Entry{
Name: "approve",
Inputs: ParameterArray{
{Type: "address"},
{Type: "uint256"},
},
}

d, err := hex.DecodeString("095ea7b3" +
"ffffffffffffffffffffffffab0974bbed8afc5212e951c8498873319d02d025" + // Valid address padded with non-zero bytes
"0000000000000000000000000000000000000000000000000000000000000064") // 100
assert.NoError(t, err)

cv, err := f.DecodeCallData(d)
assert.NoError(t, err)

address, _ := cv.Children[0].JSON()
assert.Equal(t, "\"ab0974bbed8afc5212e951c8498873319d02d025\"", string(address))
assert.Equal(t, "100", cv.Children[1].Value.(*big.Int).String())
}
4 changes: 2 additions & 2 deletions pkg/abi/typecomponents.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright © 2023 Kaleido, Inc.
// Copyright © 2024 Kaleido, Inc.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down Expand Up @@ -227,7 +227,7 @@ var (
},
jsonEncodingType: JSONEncodingTypeBytes,
encodeABIData: encodeABIUnsignedInteger,
decodeABIData: decodeABIUnsignedInt,
decodeABIData: decodeABIAddress,
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @SamMayWork - I think I need to understand this better.
Maybe I need to work through this from base principals.

decodeABIUnsignedInt should be processing exactly 20 bytes.

The ABI specification states that address is processed identically to uint256.

So on the face of it, your change seems to sidestep a bug in decodeABIUnsignedInt by introducing a new function... but I don't understand why.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, skipping working here before writing comment.
address should be processed identically to uint160

So I think the point here is that uintXXX processing is not correctly trimming before processing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fair point - address is just a unit256 under the hood, so this fixed the problem seen on the BSC chain here, but if someone padded a normal unit256 with non-zero bytes, we'd actually hit the same issue.

Taking the fixed code and moving it into the baseline unit256 function...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This issue is causing golang to panic

Then just as important. A uint256 should not panic the VM, including ffffffffffffffffffffffffab0974bbed8afc5212e951c8498873319d02d025 (which to my knowledge is a perfectly valid uint256)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I'm expecting to see @SamMayWork :

  • Processing of ffffffffffffffffffffffffab0974bbed8afc5212e951c8498873319d02d025 as a uint160 resulting in 0xab0974bbed8afc5212e951c8498873319d02d025 as a number
  • Processing of ffffffffffffffffffffffffab0974bbed8afc5212e951c8498873319d02d025 as a uint8 resulting in 0x25 as a number
  • Processing of ffffffffffffffffffffffffab0974bbed8afc5212e951c8498873319d02d025 as a uint256 processing 0xffffffffffffffffffffffffab0974bbed8afc5212e951c8498873319d02d025 as a number
  • Processing of ffffffffffffffffffffffffab0974bbed8afc5212e951c8498873319d02d025 as an address processing 0xab0974bbed8afc5212e951c8498873319d02d025 with the semantic that uint160 and address are equivalent at this layer of code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be resolved by 711fd0f

ElementaryTypeBool = registerElementaryType(elementaryTypeInfo{
name: BaseTypeBool,
Expand Down
Loading