From ca5a53775bf8e16edcd614b89038a94e710e3f64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Sat, 16 Sep 2023 20:18:39 -0700 Subject: [PATCH 1/5] add reproducer for private capability injection --- runtime/runtime_test.go | 144 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index e32394af78..17c53ed1a4 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -9311,3 +9311,147 @@ func TestRuntimeWrappedErrorHandling(t *testing.T) { // It can NOT be an internal error. assertRuntimeErrorIsUserError(t, err) } + +func TestRuntimeInvalidWrappedPrivateCapability(t *testing.T) { + + t.Parallel() + + runtime := newTestInterpreterRuntime() + runtime.defaultConfig.AtreeValidationEnabled = false + runtime.defaultConfig.AccountLinkingEnabled = true + + address := common.MustBytesToAddress([]byte{0x1}) + + linkTx := []byte(` + #allowAccountLinking + + transaction { + prepare(acct: AuthAccount) { + acct.linkAccount(/private/foo) + } + } + `) + + helperContract := []byte(` + pub contract Foo { + pub struct Thing { + pub let cap: Capability + + init(cap: Capability) { + self.cap = cap + } + } + } + `) + + getCapScript := []byte(` + import Foo from 0x1 + + pub fun main(addr: Address): AnyStruct { + let acct = getAuthAccount(addr) + let authCap = acct.getCapability<&AuthAccount>(/private/foo) + return Foo.Thing(cap: authCap) + } + `) + + attackTx := []byte(` + import Foo from 0x1 + + transaction(thing: Foo.Thing) { + prepare(_: AuthAccount) { + thing.cap.borrow<&AuthAccount>()!.save("Hello, World", to: /storage/attack) + } + } + `) + + deploy := DeploymentTransaction("Foo", helperContract) + + var accountCode []byte + var events []cadence.Event + + runtimeInterface := &testRuntimeInterface{ + getCode: func(_ Location) (bytes []byte, err error) { + return accountCode, nil + }, + storage: newTestLedger(nil, nil), + getSigningAccounts: func() ([]Address, error) { + return []Address{address}, nil + }, + resolveLocation: singleIdentifierLocationResolver(t), + getAccountContractCode: func(_ common.AddressLocation) (code []byte, err error) { + return accountCode, nil + }, + updateAccountContractCode: func(_ common.AddressLocation, code []byte) error { + accountCode = code + return nil + }, + emitEvent: func(event cadence.Event) error { + events = append(events, event) + return nil + }, + decodeArgument: func(b []byte, t cadence.Type) (cadence.Value, error) { + return jsoncdc.Decode(nil, b) + }, + } + + nextTransactionLocation := newTransactionLocationGenerator() + nextScriptLocation := newScriptLocationGenerator() + + // Deploy + + err := runtime.ExecuteTransaction( + Script{ + Source: linkTx, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Deploy helper contract + + err = runtime.ExecuteTransaction( + Script{ + Source: deploy, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Get Cap + + cap, err := runtime.ExecuteScript( + Script{ + Source: getCapScript, + Arguments: encodeArgs([]cadence.Value{ + cadence.BytesToAddress([]byte{0x1}), + }), + }, + Context{ + Interface: runtimeInterface, + Location: nextScriptLocation(), + }, + ) + require.NoError(t, err) + + // Attack + + err = runtime.ExecuteTransaction( + Script{ + Source: attackTx, + Arguments: encodeArgs([]cadence.Value{ + cap, + }), + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.Error(t, err) +} From bf07e96abee888f7f7cba73288c2e684700afe45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Sat, 16 Sep 2023 20:28:28 -0700 Subject: [PATCH 2/5] check composite fields are importable --- runtime/interpreter/value.go | 39 ++++++++++++++++++++++++++++++------ runtime/runtime_test.go | 5 ++++- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index 1d0c6a847b..8a7aa890d2 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -16395,16 +16395,22 @@ func (v *CompositeValue) Accept(interpreter *Interpreter, visitor Visitor) { return } - v.ForEachField(interpreter, func(_ string, value Value) { + v.ForEachField(interpreter, func(_ string, value Value) (resume bool) { value.Accept(interpreter, visitor) + + // continue iteration + return true }) } // Walk iterates over all field values of the composite value. // It does NOT walk the computed field or functions! func (v *CompositeValue) Walk(interpreter *Interpreter, walkChild func(Value)) { - v.ForEachField(interpreter, func(_ string, value Value) { + v.ForEachField(interpreter, func(_ string, value Value) (resume bool) { walkChild(value) + + // continue iteration + return true }) } @@ -16423,9 +16429,27 @@ func (v *CompositeValue) StaticType(interpreter *Interpreter) StaticType { } func (v *CompositeValue) IsImportable(inter *Interpreter) bool { + // Check type is importable staticType := v.StaticType(inter) semaType := inter.MustConvertStaticToSemaType(staticType) - return semaType.IsImportable(map[*sema.Member]bool{}) + if !semaType.IsImportable(map[*sema.Member]bool{}) { + return false + } + + // Check all field values are importable + importable := true + v.ForEachField(inter, func(_ string, value Value) (resume bool) { + if !value.IsImportable(inter) { + importable = false + // stop iteration + return false + } + + // continue iteration + return true + }) + + return importable } func (v *CompositeValue) IsDestroyed() bool { @@ -17452,14 +17476,17 @@ func (v *CompositeValue) GetOwner() common.Address { // ForEachField iterates over all field-name field-value pairs of the composite value. // It does NOT iterate over computed fields and functions! -func (v *CompositeValue) ForEachField(gauge common.MemoryGauge, f func(fieldName string, fieldValue Value)) { +func (v *CompositeValue) ForEachField( + gauge common.MemoryGauge, + f func(fieldName string, fieldValue Value) (resume bool), +) { err := v.dictionary.Iterate(func(key atree.Value, value atree.Value) (resume bool, err error) { - f( + resume = f( string(key.(StringAtreeValue)), MustConvertStoredValue(gauge, value), ) - return true, nil + return }) if err != nil { panic(errors.NewExternalError(err)) diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 17c53ed1a4..8f1af02fb3 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -9453,5 +9453,8 @@ func TestRuntimeInvalidWrappedPrivateCapability(t *testing.T) { Location: nextTransactionLocation(), }, ) - require.Error(t, err) + RequireError(t, err) + + var argumentNotImportableErr *ArgumentNotImportableError + require.ErrorAs(t, err, &argumentNotImportableErr) } From 3b91ff2cf924783291185fe9471def565e12594f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Sat, 16 Sep 2023 20:45:28 -0700 Subject: [PATCH 3/5] check simple composite fields are importable --- runtime/interpreter/simplecompositevalue.go | 35 ++++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/runtime/interpreter/simplecompositevalue.go b/runtime/interpreter/simplecompositevalue.go index ca53a6cec9..42ab2e928e 100644 --- a/runtime/interpreter/simplecompositevalue.go +++ b/runtime/interpreter/simplecompositevalue.go @@ -78,18 +78,25 @@ func (v *SimpleCompositeValue) Accept(interpreter *Interpreter, visitor Visitor) // ForEachField iterates over all field-name field-value pairs of the composite value. // It does NOT iterate over computed fields and functions! -func (v *SimpleCompositeValue) ForEachField(_ *Interpreter, f func(fieldName string, fieldValue Value)) { +func (v *SimpleCompositeValue) ForEachField( + f func(fieldName string, fieldValue Value) (resume bool), +) { for _, fieldName := range v.FieldNames { fieldValue := v.Fields[fieldName] - f(fieldName, fieldValue) + if !f(fieldName, fieldValue) { + break + } } } // Walk iterates over all field values of the composite value. // It does NOT walk the computed fields and functions! -func (v *SimpleCompositeValue) Walk(interpreter *Interpreter, walkChild func(Value)) { - v.ForEachField(interpreter, func(_ string, fieldValue Value) { +func (v *SimpleCompositeValue) Walk(_ *Interpreter, walkChild func(Value)) { + v.ForEachField(func(_ string, fieldValue Value) (resume bool) { walkChild(fieldValue) + + // continue iteration + return true }) } @@ -98,9 +105,27 @@ func (v *SimpleCompositeValue) StaticType(_ *Interpreter) StaticType { } func (v *SimpleCompositeValue) IsImportable(inter *Interpreter) bool { + // Check type is importable staticType := v.StaticType(inter) semaType := inter.MustConvertStaticToSemaType(staticType) - return semaType.IsImportable(map[*sema.Member]bool{}) + if !semaType.IsImportable(map[*sema.Member]bool{}) { + return false + } + + // Check all field values are importable + importable := true + v.ForEachField(func(_ string, value Value) (resume bool) { + if !value.IsImportable(inter) { + importable = false + // stop iteration + return false + } + + // continue iteration + return true + }) + + return importable } func (v *SimpleCompositeValue) GetMember( From f6ad29e903c61f64aa9067470f508f4fe92028ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Sat, 16 Sep 2023 21:19:53 -0700 Subject: [PATCH 4/5] add more test cases --- runtime/program_params_validation_test.go | 143 ++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/runtime/program_params_validation_test.go b/runtime/program_params_validation_test.go index a8781851dc..bc2f464e1e 100644 --- a/runtime/program_params_validation_test.go +++ b/runtime/program_params_validation_test.go @@ -1141,4 +1141,147 @@ func TestRuntimeTransactionParameterTypeValidation(t *testing.T) { var entryPointErr *InvalidEntryPointArgumentError require.ErrorAs(t, err, &entryPointErr) }) + + t.Run("Invalid private cap in struct", func(t *testing.T) { + t.Parallel() + + contracts := map[common.AddressLocation][]byte{ + { + Address: common.MustBytesToAddress([]byte{0x1}), + Name: "C", + }: []byte(` + pub contract C { + pub struct S { + pub let cap: Capability + + init(cap: Capability) { + self.cap = cap + } + } + } + `), + } + + script := ` + import C from 0x1 + + transaction(arg: C.S) {} + ` + + address := common.MustBytesToAddress([]byte{0x1}) + + path, err := cadence.NewPath(common.PathDomainPrivate, "foo") + require.NoError(t, err) + + capability := cadence.NewPathCapability( + cadence.Address(address), + path, + cadence.NewReferenceType(false, cadence.AuthAccountType{}), + ) + + arg := cadence.Struct{ + StructType: &cadence.StructType{ + Location: common.AddressLocation{ + Address: address, + Name: "C", + }, + QualifiedIdentifier: "C.S", + Fields: []cadence.Field{ + { + Identifier: "cap", + Type: &cadence.CapabilityType{}, + }, + }, + }, + Fields: []cadence.Value{ + capability, + }, + } + + err = executeTransaction(t, script, contracts, arg) + expectRuntimeError(t, err, &ArgumentNotImportableError{}) + }) + + t.Run("Invalid private cap in array", func(t *testing.T) { + t.Parallel() + + script := ` + transaction(arg: [AnyStruct]) {} + ` + + address := common.MustBytesToAddress([]byte{0x1}) + + path, err := cadence.NewPath(common.PathDomainPrivate, "foo") + require.NoError(t, err) + + capability := cadence.NewPathCapability( + cadence.Address(address), + path, + cadence.NewReferenceType(false, cadence.AuthAccountType{}), + ) + + arg := cadence.Array{ + ArrayType: cadence.NewVariableSizedArrayType(cadence.AnyStructType{}), + Values: []cadence.Value{ + capability, + }, + } + + err = executeTransaction(t, script, nil, arg) + expectRuntimeError(t, err, &ArgumentNotImportableError{}) + }) + + t.Run("Invalid private cap in optional", func(t *testing.T) { + t.Parallel() + + script := ` + transaction(arg: AnyStruct?) {} + ` + + address := common.MustBytesToAddress([]byte{0x1}) + + path, err := cadence.NewPath(common.PathDomainPrivate, "foo") + require.NoError(t, err) + + capability := cadence.NewPathCapability( + cadence.Address(address), + path, + cadence.NewReferenceType(false, cadence.AuthAccountType{}), + ) + + arg := cadence.NewOptional(capability) + + err = executeTransaction(t, script, nil, arg) + expectRuntimeError(t, err, &ArgumentNotImportableError{}) + }) + + t.Run("Invalid private cap in dictionary value", func(t *testing.T) { + t.Parallel() + + script := ` + transaction(arg: {String: AnyStruct}) {} + ` + + address := common.MustBytesToAddress([]byte{0x1}) + + path, err := cadence.NewPath(common.PathDomainPrivate, "foo") + require.NoError(t, err) + + capability := cadence.NewPathCapability( + cadence.Address(address), + path, + cadence.NewReferenceType(false, cadence.AuthAccountType{}), + ) + + arg := cadence.NewDictionary([]cadence.KeyValuePair{ + { + Key: cadence.String("cap"), + Value: capability, + }, + }) + + err = executeTransaction(t, script, nil, arg) + expectRuntimeError(t, err, &ArgumentNotImportableError{}) + }) + } From 9d552fcbb25563d548512692f66c85d701a4649c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 18 Sep 2023 09:22:27 -0700 Subject: [PATCH 5/5] adjust test --- runtime/tests/interpreter/values_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/runtime/tests/interpreter/values_test.go b/runtime/tests/interpreter/values_test.go index 68233b183d..31c4b61df3 100644 --- a/runtime/tests/interpreter/values_test.go +++ b/runtime/tests/interpreter/values_test.go @@ -928,11 +928,14 @@ func TestRandomCompositeValueOperations(t *testing.T) { t.Run("iterate", func(t *testing.T) { fieldCount := 0 - testComposite.ForEachField(inter, func(name string, value interpreter.Value) { + testComposite.ForEachField(inter, func(name string, value interpreter.Value) (resume bool) { orgValue, ok := orgFields[name] require.True(t, ok) utils.AssertValuesEqual(t, inter, orgValue, value) fieldCount++ + + // continue iteration + return true }) assert.Equal(t, len(orgFields), fieldCount)