Skip to content

Commit

Permalink
Increase coverage in AVM code. Bug fix in account fields of tx_field
Browse files Browse the repository at this point in the history 
Intended to just increase coverage in AVM, but doing so found a
bug. (Go figure.)  tx_field should only allow actual addresses for
address fields, rather than low integers as references from the
Account array.
  • Loading branch information
@jannotti
jannotti committed Sep 4, 2021
1 parent e32b2c2 commit 01a168e
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 6 deletions.
30 changes: 24 additions & 6 deletions data/transactions/logic/eval.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3342,6 +3342,21 @@ func opTxBegin(cx *EvalContext) {
}
}

// availableAccount is used instead of accountReference for more recent opcodes
// that don't need (or want!) to allow low numbers to represent the account at
// that index in Accounts array.
func (cx *EvalContext) availableAccount(sv stackValue) (basics.Address, error) {
if sv.argType() != StackBytes {
return basics.Address{}, fmt.Errorf("not an address")
}

addr, _, err := cx.accountReference(sv)
return addr, err
}

// availableAsset is used instead of asaReference for more recent opcodes that
// don't need (or want!) to allow low numbers to represent the asset at that
// index in ForeignAssets array.
func (cx *EvalContext) availableAsset(sv stackValue) (basics.AssetIndex, error) {
aid, err := sv.uint()
if err != nil {
Expand All @@ -3366,6 +3381,9 @@ func opTxField(cx *EvalContext) {
sv := cx.stack[last]
switch field {
case Type:
if sv.Bytes == nil {
cx.err = fmt.Errorf("Type arg not a byte array")
}
cx.subtxn.Txn.Type = protocol.TxType(sv.Bytes)
case TypeEnum:
var i uint64
Expand All @@ -3375,7 +3393,7 @@ func opTxField(cx *EvalContext) {
}

case Sender:
cx.subtxn.Txn.Sender, _, cx.err = cx.accountReference(sv)
cx.subtxn.Txn.Sender, cx.err = cx.availableAccount(sv)
case Fee:
cx.subtxn.Txn.Fee.Raw, cx.err = sv.uint()
// FirstValid, LastValid unsettable: no motivation
Expand All @@ -3388,22 +3406,22 @@ func opTxField(cx *EvalContext) {
// KeyReg not allowed yet, so no fields settable

case Receiver:
cx.subtxn.Txn.Receiver, _, cx.err = cx.accountReference(sv)
cx.subtxn.Txn.Receiver, cx.err = cx.availableAccount(sv)
case Amount:
cx.subtxn.Txn.Amount.Raw, cx.err = sv.uint()
case CloseRemainderTo:
cx.subtxn.Txn.CloseRemainderTo, _, cx.err = cx.accountReference(sv)
cx.subtxn.Txn.CloseRemainderTo, cx.err = cx.availableAccount(sv)

case XferAsset:
cx.subtxn.Txn.XferAsset, cx.err = cx.availableAsset(sv)
case AssetAmount:
cx.subtxn.Txn.AssetAmount, cx.err = sv.uint()
case AssetSender:
cx.subtxn.Txn.AssetSender, _, cx.err = cx.accountReference(sv)
cx.subtxn.Txn.AssetSender, cx.err = cx.availableAccount(sv)
case AssetReceiver:
cx.subtxn.Txn.AssetReceiver, _, cx.err = cx.accountReference(sv)
cx.subtxn.Txn.AssetReceiver, cx.err = cx.availableAccount(sv)
case AssetCloseTo:
cx.subtxn.Txn.AssetCloseTo, _, cx.err = cx.accountReference(sv)
cx.subtxn.Txn.AssetCloseTo, cx.err = cx.availableAccount(sv)

// acfg likely next

Expand Down
24 changes: 24 additions & 0 deletions data/transactions/logic/evalAppTxn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ func TestActionTypes(t *testing.T) {
testApp(t, "tx_begin; tx_submit; int 1;", ep, "Invalid inner transaction type")
// bad type
testApp(t, "tx_begin; byte \"pya\"; tx_field Type; tx_submit; int 1;", ep, "Invalid inner transaction type")
// mixed up the int form for the byte form
testApp(t, obfuscate("tx_begin; int pay; tx_field Type; tx_submit; int 1;"), ep, "Type arg not a byte array")
// or vice versa
testApp(t, obfuscate("tx_begin; byte \"pay\"; tx_field TypeEnum; tx_submit; int 1;"), ep, "not a uint64")

// good types, not alllowed yet
testApp(t, "tx_begin; byte \"keyreg\"; tx_field Type; tx_submit; int 1;", ep, "Invalid inner transaction type")
Expand Down Expand Up @@ -57,6 +61,26 @@ func TestActionTypes(t *testing.T) {
testApp(t, "tx_begin; int pay; tx_field TypeEnum; tx_submit; int 1;", ep)
}

func TestFieldTypes(t *testing.T) {
ep, _ := makeSampleEnv()
testApp(t, "tx_begin; byte \"pay\"; tx_field Sender;", ep, "not an address")
testApp(t, obfuscate("tx_begin; int 7; tx_field Receiver;"), ep, "not an address")
testApp(t, "tx_begin; byte \"\"; tx_field CloseRemainderTo;", ep, "not an address")
testApp(t, "tx_begin; byte \"\"; tx_field AssetSender;", ep, "not an address")
// can't really tell if it's an addres, so 32 bytes gets further
testApp(t, "tx_begin; byte \"01234567890123456789012345678901\"; tx_field AssetReceiver;",
ep, "invalid Account reference")
// but a b32 string rep is not an account
testApp(t, "tx_begin; byte \"GAYTEMZUGU3DOOBZGAYTEMZUGU3DOOBZGAYTEMZUGU3DOOBZGAYZIZD42E\"; tx_field AssetCloseTo;",
ep, "not an address")

testApp(t, obfuscate("tx_begin; byte \"pay\"; tx_field Fee;"), ep, "not a uint64")
testApp(t, obfuscate("tx_begin; byte 0x01; tx_field Amount;"), ep, "not a uint64")
testApp(t, obfuscate("tx_begin; byte 0x01; tx_field XferAsset;"), ep, "not a uint64")
testApp(t, obfuscate("tx_begin; byte 0x01; tx_field AssetAmount;"), ep, "not a uint64")

}

func TestAppPay(t *testing.T) {
pay := `
tx_begin
Expand Down
15 changes: 15 additions & 0 deletions data/transactions/logic/eval_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4573,17 +4573,21 @@ func TestShifts(t *testing.T) {
partitiontest.PartitionTest(t)

t.Parallel()
testAccepts(t, "int 1; int 0; shl; int 1; ==", 4)
testAccepts(t, "int 1; int 1; shl; int 2; ==", 4)
testAccepts(t, "int 1; int 2; shl; int 4; ==", 4)
testAccepts(t, "int 3; int 2; shl; int 12; ==", 4)
testAccepts(t, "int 2; int 63; shl; int 0; ==", 4)

testAccepts(t, "int 3; int 0; shr; int 3; ==", 4)
testAccepts(t, "int 1; int 1; shr; int 0; ==", 4)
testAccepts(t, "int 1; int 2; shr; int 0; ==", 4)
testAccepts(t, "int 3; int 1; shr; int 1; ==", 4)
testAccepts(t, "int 96; int 3; shr; int 12; ==", 4)
testAccepts(t, "int 8756675; int 63; shr; int 0; ==", 4)

testPanics(t, "int 8756675; int 64; shr; int 0; ==", 4)
testPanics(t, "int 8756675; int 64; shl; int 0; ==", 4)
}

func TestSqrt(t *testing.T) {
Expand Down Expand Up @@ -4623,6 +4627,10 @@ func TestExp(t *testing.T) {
testAccepts(t, "int 3; int 1; exp; int 3; ==", 4)
testAccepts(t, "int 96; int 3; exp; int 884736; ==", 4)
testPanics(t, "int 96; int 15; exp; int 884736; >", 4)

// These seem the same but check different code paths
testPanics(t, "int 2; int 64; exp; pop; int 1", 4)
testPanics(t, "int 4; int 32; exp; pop; int 1", 4)
}

func TestExpw(t *testing.T) {
Expand All @@ -4639,6 +4647,10 @@ func TestExpw(t *testing.T) {
testPanics(t, "int 64; int 22; expw; pop; pop; int 1", 4) // (2^6)^22 = 2^132

testAccepts(t, "int 97; int 15; expw; int 10271255586529954209; ==; assert; int 34328615749; ==;", 4)

testPanics(t, "int 2; int 128; expw; pop; pop; int 1", 4) // 2^128 is too big
// looks the same, but different code path
testPanics(t, "int 4; int 64; expw; pop; pop; int 1", 4) // 2^128 is too big
}

func TestBitLen(t *testing.T) {
Expand Down Expand Up @@ -4744,6 +4756,9 @@ func TestBytesBits(t *testing.T) {

testAccepts(t, "int 3; bzero; byte 0x000000; ==", 4)
testAccepts(t, "int 33; bzero; byte 0x000000000000000000000000000000000000000000000000000000000000000000; ==", 4)

testAccepts(t, "int 4096; bzero; len; int 4096; ==", 4)
testPanics(t, "int 4097; bzero; len; int 4097; ==", 4)
}

func TestBytesConversions(t *testing.T) {
Expand Down

0 comments on commit 01a168e

Please sign in to comment.