Merge pull request #5469 from Algo-devops-service/relstable3.16.2

algorand · Jun 14, 2023 · be42c82 · be42c82
2 parents 17e2f62 + 385fa9b
commit be42c82
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 2 deletions.
diff --git a/buildnumber.dat b/buildnumber.dat
@@ -1 +1 @@
-1
+2
diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go
@@ -4590,6 +4590,13 @@ func opAppLocalPut(cx *EvalContext) error {
 		return err
 	}
 
+	// The version check is overkill, but makes very clear we don't change old
+	// programs. The test here is to ensure that we didn't get access to the
+	// address from another txn, but don't have access to the local state.
+	if cx.version >= sharedResourcesVersion && !cx.allowsLocals(addr, cx.appID) {
+		return fmt.Errorf("unavailable Local State %s x %d", addr, cx.appID)
+	}
+
 	// if writing the same value, don't record in EvalDelta, matching ledger
 	// behavior with previous BuildEvalDelta mechanism
 	etv, ok, err := cx.Ledger.GetLocal(addr, cx.appID, key, accountIdx)
@@ -4678,6 +4685,13 @@ func opAppLocalDel(cx *EvalContext) error {
 		return err
 	}
 
+	// The version check is overkill, but makes very clear we don't change old
+	// programs. The test here is to ensure that we didn't get access to the
+	// address from another txn, but don't have access to the local state.
+	if cx.version >= sharedResourcesVersion && !cx.allowsLocals(addr, cx.appID) {
+		return fmt.Errorf("unavailable Local State %s x %d", addr, cx.appID)
+	}
+
 	// if deleting a non-existent value, don't record in EvalDelta, matching
 	// ledger behavior with previous BuildEvalDelta mechanism
 	if _, ok, err := cx.Ledger.GetLocal(addr, cx.appID, key, accountIdx); ok {
@@ -4792,7 +4806,7 @@ func (cx *EvalContext) resolveApp(ref uint64) (aid basics.AppIndex, err error) {
 // and the App, taking access rules into account.  It has the funny side job of
 // also reporting which "slot" the address appears in, if it is in txn.Accounts
 // (or is the Sender, which yields 0). But it only needs to do this funny side
-// job in certainly old versions that need the slot index while doing a lookup.
+// job in certain old versions that need the slot index while doing a lookup.
 func (cx *EvalContext) localsReference(account stackValue, ref uint64) (basics.Address, basics.AppIndex, uint64, error) {
 	if cx.version >= sharedResourcesVersion {
 		addr, _, err := cx.resolveAccount(account)

diff --git a/data/transactions/logic/evalStateful_test.go b/data/transactions/logic/evalStateful_test.go
@@ -446,6 +446,10 @@ func testAppsBytes(t *testing.T, programs [][]byte, ep *EvalParams, expected ...
 			} else {
 				testAppFull(t, program, i, appID, ep)
 			}
+		} else {
+			if len(expected) > 0 && expected[0].l == i {
+				require.Failf(t, "testAppsBytes used incorrectly.", "No error can happen in txn %d. Not an app.", i)
+			}
 		}
 	}
 }

diff --git a/data/transactions/logic/resources_test.go b/data/transactions/logic/resources_test.go
@@ -58,6 +58,12 @@ func TestAppSharing(t *testing.T) {
 		Sender:        basics.Address{1, 2, 3, 4},
 	}
 
+	pay1 := txntest.Txn{
+		Type:     protocol.PaymentTx,
+		Sender:   basics.Address{5, 5, 5, 5},
+		Receiver: basics.Address{6, 6, 6, 6},
+	}
+
 	getSchema := "int 500; app_params_get AppGlobalNumByteSlice; !; assert; pop; int 1"
 	// In v8, the first tx can read app params of 500, because it's in its
 	// foreign array, but the second can't
@@ -127,6 +133,11 @@ func TestAppSharing(t *testing.T) {
 	noop := `int 1`
 	sources := []string{noop, putLocal}
 	appl1.ApplicationArgs = [][]byte{appl0.Sender[:]} // tx1 will try to modify local state exposed in tx0
+	// appl0.Sender is available, but 901's local state for it isn't (only 900 is, since 900 was called in tx0)
+	logic.TestApps(t, sources, txntest.Group(&appl0, &appl1), 9, ledger,
+		logic.Exp(1, "unavailable Local State "+appl0.Sender.String()))
+	// Add 901 to tx0's ForeignApps, and it works
+	appl0.ForeignApps = append(appl0.ForeignApps, 901) // well, it will after we opt in
 	logic.TestApps(t, sources, txntest.Group(&appl0, &appl1), 9, ledger,
 		logic.Exp(1, "account "+appl0.Sender.String()+" is not opted into 901"))
 	ledger.NewLocals(appl0.Sender, 901) // opt in
@@ -153,6 +164,31 @@ func TestAppSharing(t *testing.T) {
 	sources = []string{"", "", "gtxn 1 Accounts 1; gtxn 0 Applications 0; byte 0xAA; app_local_get_ex"}
 	logic.TestApps(t, sources, txntest.Group(&appl0, &appl1, &appl2), 9, nil,
 		logic.Exp(2, "unavailable Local State")) // note that the error message is for Locals, not specialized
+
+	// try to do a put on local state of the account in tx1, but tx0 ought not have access to that local state
+	ledger.NewAccount(pay1.Receiver, 200_000)
+	ledger.NewLocals(pay1.Receiver, 900) // opt in
+	sources = []string{`gtxn 1 Receiver; byte "key"; byte "val"; app_local_put; int 1`}
+	logic.TestApps(t, sources, txntest.Group(&appl0, &pay1), 9, ledger,
+		logic.Exp(0, "unavailable Local State "+pay1.Receiver.String()))
+
+	// same for app_local_del
+	sources = []string{`gtxn 1 Receiver; byte "key"; app_local_del; int 1`}
+	logic.TestApps(t, sources, txntest.Group(&appl0, &pay1), 9, ledger,
+		logic.Exp(0, "unavailable Local State "+pay1.Receiver.String()))
+
+	// now, use an app call in tx1, with 900 in the foreign apps, so the local state is available
+	appl1.ForeignApps = append(appl1.ForeignApps, 900)
+	ledger.NewLocals(appl1.Sender, 900) // opt in
+	sources = []string{`gtxn 1 Sender; byte "key"; byte "val"; app_local_put; int 1`}
+	logic.TestApps(t, sources, txntest.Group(&appl0, &appl1), 9, ledger)
+	logic.TestApps(t, sources, txntest.Group(&appl0, &appl1), 8, ledger, // 8 doesn't share the account
+		logic.Exp(0, "invalid Account reference "+appl1.Sender.String()))
+	// same for app_local_del
+	sources = []string{`gtxn 1 Sender; byte "key"; app_local_del; int 1`}
+	logic.TestApps(t, sources, txntest.Group(&appl0, &appl1), 9, ledger)
+	logic.TestApps(t, sources, txntest.Group(&appl0, &appl1), 8, ledger, // 8 doesn't share the account
+		logic.Exp(0, "invalid Account reference "+appl1.Sender.String()))
 }
 
 // TestBetterLocalErrors confirms that we get specific errors about the missing