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 buffer offset src EVM test #101

Merged
merged 2 commits into from
Feb 2, 2024
Merged

Fix buffer offset src EVM test #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad51046
GeneralStateTests/stMemoryTest/bufferSrcOffset.json test fix (#84)
cokicm Feb 2, 2024
0ac610d
Linter fix
Stefan-Ethernal Feb 2, 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
41 changes: 22 additions & 19 deletions state/runtime/evm/instructions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,41 +809,44 @@ func opReturnDataCopy(c *state) {
return
}

memOffset := c.pop()
dataOffset := c.pop()
length := c.pop()
var (
memOffset = c.pop()
dataOffset = c.pop()
length = c.pop()
)

if !dataOffset.IsUint64() {
// Check if:
// 1. the dataOffset is uint64 (overflow check)
// 2. the sum of dataOffset and length overflows uint64
// 3. the length of return data has enough space to receive offset + length bytes
end := new(big.Int).Add(dataOffset, length)
endAddress := end.Uint64()

if !dataOffset.IsUint64() ||
!end.IsUint64() ||
uint64(len(c.returnData)) < endAddress {
c.exit(errReturnDataOutOfBounds)

return
}

// if length is 0, return immediately since no need for the data copying nor memory allocation
if length.Sign() == 0 || !c.allocateMemory(memOffset, length) {
return
}

ulength := length.Uint64()
if !c.consumeGas(((ulength + 31) / 32) * copyGas) {
if length.Sign() == 0 {
return
}

dataEnd := length.Add(dataOffset, length)
if !dataEnd.IsUint64() {
c.exit(errReturnDataOutOfBounds)

if !c.allocateMemory(memOffset, length) {
// Error code is set inside the allocateMemory call
return
}

dataEndIndex := dataEnd.Uint64()
if uint64(len(c.returnData)) < dataEndIndex {
c.exit(errReturnDataOutOfBounds)

ulength := length.Uint64()
if !c.consumeGas(((ulength + 31) / 32) * copyGas) {
// Error code is set inside the consumeGas
return
}

data := c.returnData[dataOffset.Uint64():dataEndIndex]
data := c.returnData[dataOffset.Uint64():endAddress]
copy(c.memory[memOffset.Uint64():memOffset.Uint64()+ulength], data)
}

Expand Down
164 changes: 123 additions & 41 deletions state/runtime/evm/instructions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1726,6 +1726,16 @@ func TestCreate(t *testing.T) {
func Test_opReturnDataCopy(t *testing.T) {
t.Parallel()

// Positive number that does not fit in uint64 (math.MaxUint64 + 1)
largeNumber := "18446744073709551616"
bigIntValue := new(big.Int)
bigIntValue.SetString(largeNumber, 10)

// Positive number that does fit in uint64 but multiplied by two does not
largeNumber2 := "18446744073709551615"
bigIntValue2 := new(big.Int)
bigIntValue2.SetString(largeNumber2, 10)

tests := []struct {
name string
config *chain.ForksInTime
Expand Down Expand Up @@ -1755,7 +1765,8 @@ func Test_opReturnDataCopy(t *testing.T) {
big.NewInt(0), // dataOffset
big.NewInt(-1), // memOffset
},
sp: 3,
sp: 3,
returnData: []byte{0xff},
},
resultState: &state{
config: &allEnabledForks,
Expand All @@ -1764,9 +1775,10 @@ func Test_opReturnDataCopy(t *testing.T) {
big.NewInt(0),
big.NewInt(-1),
},
sp: 0,
stop: true,
err: errReturnDataOutOfBounds,
sp: 0,
returnData: []byte{0xff},
stop: true,
err: errReturnDataOutOfBounds,
},
},
{
Expand Down Expand Up @@ -1800,21 +1812,23 @@ func Test_opReturnDataCopy(t *testing.T) {
initState: &state{
stack: []*big.Int{
big.NewInt(-1), // length
big.NewInt(0), // dataOffset
big.NewInt(2), // dataOffset
big.NewInt(0), // memOffset
},
sp: 3,
sp: 3,
returnData: []byte{0xff},
},
resultState: &state{
config: &allEnabledForks,
stack: []*big.Int{
big.NewInt(-1),
big.NewInt(0),
big.NewInt(2),
big.NewInt(0),
},
sp: 0,
stop: true,
err: errReturnDataOutOfBounds,
sp: 0,
returnData: []byte{0xff},
stop: true,
err: errReturnDataOutOfBounds,
},
},
{
Expand Down Expand Up @@ -1849,66 +1863,134 @@ func Test_opReturnDataCopy(t *testing.T) {
},
},
{
name: "should expand memory and copy data returnData",
// this test case also verifies that code does not panic when the length is 0 and memOffset > len(memory)
name: "should not copy data if length is zero",
config: &allEnabledForks,
initState: &state{
stack: []*big.Int{
big.NewInt(5), // length
big.NewInt(1), // dataOffset
big.NewInt(2), // memOffset
big.NewInt(0), // length
big.NewInt(0), // dataOffset
big.NewInt(4), // memOffset
},
sp: 3,
returnData: []byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06},
memory: []byte{0x11, 0x22},
gas: 20,
returnData: []byte{0x01},
memory: []byte{0x02},
},
resultState: &state{
config: &allEnabledForks,
stack: []*big.Int{
big.NewInt(0),
big.NewInt(0),
big.NewInt(4),
},
sp: 0,
returnData: []byte{0x01},
memory: []byte{0x02},
stop: false,
err: nil,
},
},
{
name: "should return error if data offset overflows uint64",
config: &allEnabledForks,
initState: &state{
stack: []*big.Int{
big.NewInt(1), // length
bigIntValue, // dataOffset
big.NewInt(-1), // memOffset
},
sp: 3,
},
resultState: &state{
config: &allEnabledForks,
stack: []*big.Int{
big.NewInt(6), // updated for end index
big.NewInt(1),
bigIntValue,
big.NewInt(-1),
},
sp: 0,
stop: true,
err: errReturnDataOutOfBounds,
},
},
{
name: "should return error if sum of data offset and length overflows uint64",
config: &allEnabledForks,
initState: &state{
stack: []*big.Int{
bigIntValue2, // length
bigIntValue2, // dataOffset
big.NewInt(-1), // memOffset
},
sp: 3,
},
resultState: &state{
config: &allEnabledForks,
stack: []*big.Int{
bigIntValue2,
bigIntValue2,
big.NewInt(-1),
},
sp: 0,
stop: true,
err: errReturnDataOutOfBounds,
},
},
{
name: "should return error if the length of return data does not have enough space to receive offset + length bytes",
config: &allEnabledForks,
initState: &state{
stack: []*big.Int{
big.NewInt(2), // length
big.NewInt(0), // dataOffset
big.NewInt(0), // memOffset
},
sp: 3,
returnData: []byte{0xff},
memory: []byte{0x0},
},
resultState: &state{
config: &allEnabledForks,
stack: []*big.Int{
big.NewInt(2),
big.NewInt(0),
big.NewInt(0),
},
sp: 0,
returnData: []byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06},
memory: append(
// 1 word (32 bytes)
[]byte{0x11, 0x22, 0x02, 0x03, 0x04, 0x05, 0x06},
make([]byte, 25)...,
),
gas: 14,
lastGasCost: 3,
currentConsumedGas: 6,
stop: false,
err: nil,
returnData: []byte{0xff},
memory: []byte{0x0},
stop: true,
err: errReturnDataOutOfBounds,
},
},
{
// this test case also verifies that code does not panic when the length is 0 and memOffset > len(memory)
name: "should not copy data if length is zero",
name: "should return error if there is no gas",
config: &allEnabledForks,
initState: &state{
stack: []*big.Int{
big.NewInt(0), // length
big.NewInt(1), // length
big.NewInt(0), // dataOffset
big.NewInt(4), // memOffset
big.NewInt(0), // memOffset
},
sp: 3,
returnData: []byte{0x01},
memory: []byte{0x02},
returnData: []byte{0xff},
memory: []byte{0x0},
gas: 0,
},
resultState: &state{
config: &allEnabledForks,
stack: []*big.Int{
big.NewInt(1),
big.NewInt(0),
big.NewInt(0),
big.NewInt(4),
},
sp: 0,
returnData: []byte{0x01},
memory: []byte{0x02},
stop: false,
err: nil,
sp: 0,
returnData: []byte{0xff},
memory: []byte{0x0},
gas: 0,
currentConsumedGas: 3,
stop: true,
err: errOutOfGas,
},
},
}
Expand Down
Loading