diff --git a/runtime/ast/conditionkind.go b/runtime/ast/conditionkind.go index fa694dcad8..da4d7d841a 100644 --- a/runtime/ast/conditionkind.go +++ b/runtime/ast/conditionkind.go @@ -52,3 +52,14 @@ func (k ConditionKind) Name() string { func (k ConditionKind) MarshalJSON() ([]byte, error) { return json.Marshal(k.String()) } + +func (k ConditionKind) Keyword() string { + switch k { + case ConditionKindPre: + return "pre" + case ConditionKindPost: + return "post" + } + + panic(errors.NewUnreachableError()) +} diff --git a/runtime/deployment_test.go b/runtime/deployment_test.go index 92687f3a49..d91d5bebdb 100644 --- a/runtime/deployment_test.go +++ b/runtime/deployment_test.go @@ -105,8 +105,6 @@ func TestRuntimeTransactionWithContractDeployment(t *testing.T) { var runtimeErr Error require.ErrorAs(t, err, &runtimeErr) - println(runtimeErr.Error()) - assert.EqualError(t, runtimeErr, expectedErrorMessage) assert.Len(t, runtimeErr.Codes, 2) diff --git a/runtime/interpreter/interpreter.go b/runtime/interpreter/interpreter.go index 33b7c35d5d..eb0eea62da 100644 --- a/runtime/interpreter/interpreter.go +++ b/runtime/interpreter/interpreter.go @@ -659,10 +659,7 @@ func (interpreter *Interpreter) VisitProgram(program *ast.Program) { var variable *Variable variable = NewVariableWithGetter(interpreter, func() Value { - var result Value - interpreter.visitVariableDeclaration(declaration, func(_ string, value Value) { - result = value - }) + result := interpreter.visitVariableDeclaration(declaration, false) // Global variables are lazily loaded. Therefore, start resource tracking also // lazily when the resource is used for the first time. @@ -895,13 +892,10 @@ func (interpreter *Interpreter) declareVariable(identifier string, value Value) func (interpreter *Interpreter) visitAssignment( transferOperation ast.TransferOperation, - targetExpression ast.Expression, targetType sema.Type, + targetGetterSetter getterSetter, targetType sema.Type, valueExpression ast.Expression, valueType sema.Type, position ast.HasPosition, ) { - // First evaluate the target, which results in a getter/setter function pair - getterSetter := interpreter.assignmentGetterSetter(targetExpression) - locationRange := LocationRange{ Location: interpreter.Location, HasPosition: position, @@ -918,7 +912,7 @@ func (interpreter *Interpreter) visitAssignment( const allowMissing = true - target := getterSetter.get(allowMissing) + target := targetGetterSetter.get(allowMissing) if _, ok := target.(NilValue); !ok && target != nil { panic(ForceAssignmentToNonNilResourceError{ @@ -933,7 +927,7 @@ func (interpreter *Interpreter) visitAssignment( transferredValue := interpreter.transferAndConvert(value, valueType, targetType, locationRange) - getterSetter.set(transferredValue) + targetGetterSetter.set(transferredValue) } // NOTE: only called for top-level composite declarations @@ -5483,10 +5477,9 @@ func (interpreter *Interpreter) withMutationPrevention(storageID atree.StorageID } } -func (interpreter *Interpreter) withResourceDestruction( +func (interpreter *Interpreter) enforceNotResourceDestruction( storageID atree.StorageID, locationRange LocationRange, - f func(), ) { _, exists := interpreter.SharedState.destroyedResources[storageID] if exists { @@ -5494,6 +5487,14 @@ func (interpreter *Interpreter) withResourceDestruction( LocationRange: locationRange, }) } +} + +func (interpreter *Interpreter) withResourceDestruction( + storageID atree.StorageID, + locationRange LocationRange, + f func(), +) { + interpreter.enforceNotResourceDestruction(storageID, locationRange) interpreter.SharedState.destroyedResources[storageID] = struct{}{} diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index 96420aa4cd..e5b5b0cbb3 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -1120,6 +1120,9 @@ func (interpreter *Interpreter) VisitCastingExpression(expression *ast.CastingEx // The failable cast may upcast to an optional type, e.g. `1 as? Int?`, so box value = interpreter.BoxOptional(locationRange, value, expectedType) + // Failable casting is a resource invalidation + interpreter.invalidateResource(value) + return NewSomeValueNonCopying(interpreter, value) case ast.OperationForceCast: diff --git a/runtime/interpreter/interpreter_statement.go b/runtime/interpreter/interpreter_statement.go index dd6090c68c..1a0a12fa1f 100644 --- a/runtime/interpreter/interpreter_statement.go +++ b/runtime/interpreter/interpreter_statement.go @@ -24,6 +24,7 @@ import ( "github.com/onflow/cadence/runtime/ast" "github.com/onflow/cadence/runtime/common" "github.com/onflow/cadence/runtime/errors" + "github.com/onflow/cadence/runtime/sema" ) func (interpreter *Interpreter) evalStatement(statement ast.Statement) StatementResult { @@ -141,66 +142,22 @@ func (interpreter *Interpreter) visitIfStatementWithVariableDeclaration( thenBlock, elseBlock *ast.Block, ) StatementResult { - // NOTE: It is *REQUIRED* that the getter for the value is used - // instead of just evaluating value expression, - // as the value may be an access expression (member access, index access), - // which implicitly removes a resource. - // - // Performing the removal from the container is essential - // (and just evaluating the expression does not perform the removal), - // because if there is a second value, - // the assignment to the value will cause an overwrite of the value. - // If the resource was not moved ou of the container, - // its contents get deleted. - - getterSetter := interpreter.assignmentGetterSetter(declaration.Value) - - const allowMissing = false - value := getterSetter.get(allowMissing) - if value == nil { - panic(errors.NewUnreachableError()) - } - - variableDeclarationTypes := interpreter.Program.Elaboration.VariableDeclarationTypes(declaration) - valueType := variableDeclarationTypes.ValueType - - if declaration.SecondValue != nil { - secondValueType := variableDeclarationTypes.SecondValueType - - interpreter.visitAssignment( - declaration.Transfer.Operation, - declaration.Value, - valueType, - declaration.SecondValue, - secondValueType, - declaration, - ) - } + value := interpreter.visitVariableDeclaration(declaration, true) if someValue, ok := value.(*SomeValue); ok { - - targetType := variableDeclarationTypes.TargetType locationRange := LocationRange{ Location: interpreter.Location, HasPosition: declaration.Value, } + innerValue := someValue.InnerValue(interpreter, locationRange) - transferredUnwrappedValue := interpreter.transferAndConvert( - innerValue, - valueType, - targetType, - locationRange, - ) interpreter.activations.PushNewWithCurrent() defer interpreter.activations.Pop() - // Assignment can also be a resource move. - interpreter.invalidateResource(innerValue) - interpreter.declareVariable( declaration.Identifier.Identifier, - transferredUnwrappedValue, + innerValue, ) return interpreter.visitBlock(thenBlock) @@ -461,18 +418,14 @@ func (interpreter *Interpreter) VisitPragmaDeclaration(_ *ast.PragmaDeclaration) // then declares the variable with the name bound to the value func (interpreter *Interpreter) VisitVariableDeclaration(declaration *ast.VariableDeclaration) StatementResult { - interpreter.visitVariableDeclaration( - declaration, - func(identifier string, value Value) { + value := interpreter.visitVariableDeclaration(declaration, false) - // NOTE: lexical scope, always declare a new variable. - // Do not find an existing variable and assign the value! + // NOTE: lexical scope, always declare a new variable. + // Do not find an existing variable and assign the value! - _ = interpreter.declareVariable( - identifier, - value, - ) - }, + _ = interpreter.declareVariable( + declaration.Identifier.Identifier, + value, ) return nil @@ -480,8 +433,8 @@ func (interpreter *Interpreter) VisitVariableDeclaration(declaration *ast.Variab func (interpreter *Interpreter) visitVariableDeclaration( declaration *ast.VariableDeclaration, - valueCallback func(identifier string, value Value), -) { + isOptionalBinding bool, +) Value { variableDeclarationTypes := interpreter.Program.Elaboration.VariableDeclarationTypes(declaration) targetType := variableDeclarationTypes.TargetType @@ -497,7 +450,7 @@ func (interpreter *Interpreter) visitVariableDeclaration( // (and just evaluating the expression does not perform the removal), // because if there is a second value, // the assignment to the value will cause an overwrite of the value. - // If the resource was not moved ou of the container, + // If the resource was not moved out of the container, // its contents get deleted. getterSetter := interpreter.assignmentGetterSetter(declaration.Value) @@ -508,33 +461,39 @@ func (interpreter *Interpreter) visitVariableDeclaration( panic(errors.NewUnreachableError()) } - // Assignment is a potential resource move. - interpreter.invalidateResource(result) - locationRange := LocationRange{ Location: interpreter.Location, HasPosition: declaration.Value, } - transferredValue := interpreter.transferAndConvert(result, valueType, targetType, locationRange) + if isOptionalBinding { + targetType = &sema.OptionalType{ + Type: targetType, + } + } - valueCallback( - declaration.Identifier.Identifier, - transferredValue, + transferredValue := interpreter.transferAndConvert( + result, + valueType, + targetType, + locationRange, ) - if declaration.SecondValue == nil { - return + // Assignment is a potential resource move. + interpreter.invalidateResource(result) + + if declaration.SecondValue != nil { + interpreter.visitAssignment( + declaration.Transfer.Operation, + getterSetter, + valueType, + declaration.SecondValue, + secondValueType, + declaration, + ) } - interpreter.visitAssignment( - declaration.Transfer.Operation, - declaration.Value, - valueType, - declaration.SecondValue, - secondValueType, - declaration, - ) + return transferredValue } func (interpreter *Interpreter) VisitAssignmentStatement(assignment *ast.AssignmentStatement) StatementResult { @@ -545,9 +504,11 @@ func (interpreter *Interpreter) VisitAssignmentStatement(assignment *ast.Assignm target := assignment.Target value := assignment.Value + getterSetter := interpreter.assignmentGetterSetter(target) + interpreter.visitAssignment( assignment.Transfer.Operation, - target, targetType, + getterSetter, targetType, value, valueType, assignment, ) diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index 3418719755..42c71267bc 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -16798,6 +16798,8 @@ func (v *CompositeValue) SetMember( v.checkInvalidatedResourceUse(locationRange) } + interpreter.enforceNotResourceDestruction(v.StorageID(), locationRange) + if config.TracingEnabled { startTime := time.Now() @@ -19412,6 +19414,11 @@ func (v *SomeValue) NeedsStoreTo(address atree.Address) bool { } func (v *SomeValue) IsResourceKinded(interpreter *Interpreter) bool { + // If the inner value is `nil`, then this is an invalidated resource. + if v.value == nil { + return true + } + return v.value.IsResourceKinded(interpreter) } diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index ebb2ef8d02..28936ecbbc 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -9504,9 +9504,6 @@ func TestNestedResourceMoveInDestructor(t *testing.T) { emitEvent: func(event cadence.Event) error { return nil }, - log: func(s string) { - fmt.Println(s) - }, } nextTransactionLocation := newTransactionLocationGenerator() @@ -9661,6 +9658,193 @@ func TestNestedResourceMoveInDestructor(t *testing.T) { require.ErrorAs(t, err, &interpreter.UseBeforeInitializationError{}) } +func TestRuntimeNestedResourceMoveWithSecondTransferInDestructor(t *testing.T) { + + t.Parallel() + + runtime := newTestInterpreterRuntime() + + signerAccount := common.MustBytesToAddress([]byte{0x1}) + + signers := []Address{signerAccount} + + accountCodes := map[Location][]byte{} + + runtimeInterface := &testRuntimeInterface{ + getCode: func(location Location) (bytes []byte, err error) { + return accountCodes[location], nil + }, + storage: newTestLedger(nil, nil), + getSigningAccounts: func() ([]Address, error) { + return signers, nil + }, + resolveLocation: singleIdentifierLocationResolver(t), + getAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + return accountCodes[location], nil + }, + updateAccountContractCode: func(location common.AddressLocation, code []byte) (err error) { + accountCodes[location] = code + return nil + }, + emitEvent: func(event cadence.Event) error { + return nil + }, + } + + nextTransactionLocation := newTransactionLocationGenerator() + + attacker := []byte(fmt.Sprintf(` + import Bar from %[1]s + + access(all) contract Foo { + pub var temp: @Bar.Vault? + init() { + self.temp <- nil + } + + pub fun doubler(_ vault: @Bar.Vault): @Bar.Vault { + destroy <- create R(<-vault) + var doubled <- self.temp <- nil + return <- doubled! + } + + pub resource R { + pub var bounty: @Bar.Vault + pub var dummy: @Bar.Vault + + init(_ v: @Bar.Vault) { + self.bounty <- v + self.dummy <- Bar.createEmptyVault() + } + + pub fun swap() { + self.bounty <-> self.dummy + } + + destroy() { + // Nested resource is moved here once + var bounty <- self.bounty.withdraw(amount: 0.0) + var throwaway <- bounty <- self.bounty + destroy throwaway + + // Nested resource is again moved here. This one should fail. + self.swap() + + var dummy <- self.dummy + + var r1 = &bounty as &Bar.Vault + + Foo.account.save(<-dummy, to: /storage/dummy) + Foo.account.save(<-bounty, to: /storage/bounty) + + var dummy2 <- Foo.account.load<@Bar.Vault>(from: /storage/dummy)! + var bounty2 <- Foo.account.load<@Bar.Vault>(from: /storage/bounty)! + + dummy2.deposit(from:<-bounty2) + Foo.temp <-! dummy2 + } + } + }`, + signerAccount.HexWithPrefix(), + )) + + bar := []byte(` + pub contract Bar { + pub resource Vault { + + // Balance of a user's Vault + // we use unsigned fixed point numbers for balances + // because they can represent decimals and do not allow negative values + pub var balance: UFix64 + + init(balance: UFix64) { + self.balance = balance + } + + pub fun withdraw(amount: UFix64): @Vault { + self.balance = self.balance - amount + return <-create Vault(balance: amount) + } + + pub fun deposit(from: @Vault) { + self.balance = self.balance + from.balance + destroy from + } + } + + pub fun createEmptyVault(): @Bar.Vault { + return <- create Bar.Vault(balance: 0.0) + } + + pub fun createVault(balance: UFix64): @Bar.Vault { + return <- create Bar.Vault(balance: balance) + } + } + `) + + // Deploy Bar + + deployVault := DeploymentTransaction("Bar", bar) + err := runtime.ExecuteTransaction( + Script{ + Source: deployVault, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Deploy Attacker + + deployAttacker := DeploymentTransaction("Foo", attacker) + + err = runtime.ExecuteTransaction( + Script{ + Source: deployAttacker, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Attack + + attackTransaction := []byte(fmt.Sprintf(` + import Foo from %[1]s + import Bar from %[1]s + + transaction { + prepare(acc: AuthAccount) { + acc.save(<- Bar.createVault(balance: 100.0), to: /storage/vault)! + var vault = acc.borrow<&Bar.Vault>(from: /storage/vault)! + var flow <- vault.withdraw(amount: 42.0) + + var doubled <- Foo.doubler(<-flow) + + destroy doubled + } + }`, + signerAccount.HexWithPrefix(), + )) + + err = runtime.ExecuteTransaction( + Script{ + Source: attackTransaction, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.UseBeforeInitializationError{}) +} + func TestNestedResourceMoveInTransaction(t *testing.T) { t.Parallel() @@ -9756,3 +9940,378 @@ func TestNestedResourceMoveInTransaction(t *testing.T) { require.NoError(t, err) } + +func TestRuntimePreconditionDuplication(t *testing.T) { + + t.Parallel() + + runtime := newTestInterpreterRuntime() + + signerAccount := common.MustBytesToAddress([]byte{0x1}) + + signers := []Address{signerAccount} + + accountCodes := map[Location][]byte{} + + runtimeInterface := &testRuntimeInterface{ + getCode: func(location Location) (bytes []byte, err error) { + return accountCodes[location], nil + }, + storage: newTestLedger(nil, nil), + getSigningAccounts: func() ([]Address, error) { + return signers, nil + }, + resolveLocation: singleIdentifierLocationResolver(t), + getAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + return accountCodes[location], nil + }, + updateAccountContractCode: func(location common.AddressLocation, code []byte) (err error) { + accountCodes[location] = code + return nil + }, + emitEvent: func(event cadence.Event) error { + return nil + }, + } + + nextTransactionLocation := newTransactionLocationGenerator() + + attacker := []byte(fmt.Sprintf(` + import Bar from %[1]s + + access(all) contract Foo { + pub var temp: @Bar.Vault? + + init() { + self.temp <- nil + } + + pub fun doubler(_ vault: @Bar.Vault): @Bar.Vault { + var r <- create R(<-vault) + r.doMagic() + destroy r + var doubled <- self.temp <- nil + return <- doubled! + } + + pub fun conditionFunction(_ unusedref: &Bar.Vault, _ v : @Bar.Vault, _ ref: &R): Bool { + // This gets called twice: once from the interface's precondition and the second + // time from the function's own precondition. We save both copies of the vault + // to the R object + if ref.counter == 0 { + ref.victim1 <-! v + ref.counter = 1 + } else { + ref.victim2 <-! v + } + return true + } + + pub resource interface RInterface{ + pub fun funcFromIface( _ v: @Bar.Vault, _ ref: &R): Void { + post { + Foo.conditionFunction(&v as &Bar.Vault, <- v, ref) + } + } + } + + pub resource R: RInterface { + pub var bounty: @Bar.Vault? + pub(set) var victim1: @Bar.Vault? + pub(set) var victim2: @Bar.Vault? + pub(set) var counter: Int + + init(_ v: @Bar.Vault) { + self.counter = 0 + self.bounty <- v + self.victim1 <- nil + self.victim2 <- nil + } + + pub fun funcFromIface(_ v: @Bar.Vault, _ ref: &R): Void { + post { + Foo.conditionFunction(&v as &Bar.Vault, <- v, ref) + } + } + + pub fun doMagic(): Void { + var origVault <- self.bounty <- nil + self.funcFromIface(<- origVault!, &self as &R) + + var v1 <- self.victim1 <- nil + var v2 <- self.victim2 <- nil + + // Following moves copied from Supun's test + var r1 = &v2 as &Bar.Vault? + Foo.account.save(<-v1!, to: /storage/v1) + Foo.account.save(<-v2!, to: /storage/v2) + var v1Reloaded <- Foo.account.load<@Bar.Vault>(from: /storage/v1)! + var v2Reloaded <- Foo.account.load<@Bar.Vault>(from: /storage/v2)! + + v1Reloaded.deposit(from:<-v2Reloaded) + Foo.temp <-! v1Reloaded + } + + destroy() { + destroy self.bounty + destroy self.victim1 + destroy self.victim2 + } + } + }`, + signerAccount.HexWithPrefix(), + )) + + bar := []byte(` + pub contract Bar { + pub resource Vault { + + // Balance of a user's Vault + // we use unsigned fixed point numbers for balances + // because they can represent decimals and do not allow negative values + pub var balance: UFix64 + + init(balance: UFix64) { + self.balance = balance + } + + pub fun withdraw(amount: UFix64): @Vault { + self.balance = self.balance - amount + return <-create Vault(balance: amount) + } + + pub fun deposit(from: @Vault) { + self.balance = self.balance + from.balance + destroy from + } + } + + pub fun createEmptyVault(): @Bar.Vault { + return <- create Bar.Vault(balance: 0.0) + } + + pub fun createVault(balance: UFix64): @Bar.Vault { + return <- create Bar.Vault(balance: balance) + } + } + `) + + // Deploy Bar + + deployVault := DeploymentTransaction("Bar", bar) + err := runtime.ExecuteTransaction( + Script{ + Source: deployVault, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Deploy Attacker + + deployAttacker := DeploymentTransaction("Foo", attacker) + + err = runtime.ExecuteTransaction( + Script{ + Source: deployAttacker, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + + RequireError(t, err) + + var checkerErr *sema.CheckerError + require.ErrorAs(t, err, &checkerErr) + + errs := checker.RequireCheckerErrors(t, checkerErr, 1) + + assert.IsType(t, &sema.InvalidInterfaceConditionResourceInvalidationError{}, errs[0]) +} + +func TestRuntimeIfLetElseBranchConfusion(t *testing.T) { + + t.Parallel() + + runtime := newTestInterpreterRuntime() + + signerAccount := common.MustBytesToAddress([]byte{0x1}) + + signers := []Address{signerAccount} + + accountCodes := map[Location][]byte{} + + runtimeInterface := &testRuntimeInterface{ + getCode: func(location Location) (bytes []byte, err error) { + return accountCodes[location], nil + }, + storage: newTestLedger(nil, nil), + getSigningAccounts: func() ([]Address, error) { + return signers, nil + }, + resolveLocation: singleIdentifierLocationResolver(t), + getAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + return accountCodes[location], nil + }, + updateAccountContractCode: func(location common.AddressLocation, code []byte) (err error) { + accountCodes[location] = code + return nil + }, + emitEvent: func(event cadence.Event) error { + return nil + }, + log: func(s string) { + fmt.Println(s) + }, + } + + nextTransactionLocation := newTransactionLocationGenerator() + + attacker := []byte(fmt.Sprintf(` + import Bar from %[1]s + + access(all) contract Foo { + pub var temp: @Bar.Vault? + init() { + self.temp <- nil + } + + pub fun doubler(_ vault: @Bar.Vault): @Bar.Vault { + destroy <- create R(<-vault) + var doubled <- self.temp <- nil + return <- doubled! + } + + pub resource R { + priv var r: @Bar.Vault? + priv var r2: @Bar.Vault? + + init(_ v: @Bar.Vault) { + self.r <- nil + self.r2 <- v + } + + destroy() { + if let dummy <- self.r <- self.r2{ + // unreachable token destroys to please checker + destroy dummy + destroy self.r + } else { + var dummy <- self.r2 + var bounty <- self.r + var r1 = &bounty as &Bar.Vault? + Foo.account.save(<-dummy!, to: /storage/dummy) + Foo.account.save(<-bounty!, to: /storage/bounty) + var dummy2 <- Foo.account.load<@Bar.Vault>(from: /storage/dummy)! + var bounty2 <- Foo.account.load<@Bar.Vault>(from: /storage/bounty)! + + dummy2.deposit(from:<-bounty2) + Foo.temp <-! dummy2 + } + } + } + }`, + signerAccount.HexWithPrefix(), + )) + + bar := []byte(` + pub contract Bar { + pub resource Vault { + + // Balance of a user's Vault + // we use unsigned fixed point numbers for balances + // because they can represent decimals and do not allow negative values + pub var balance: UFix64 + + init(balance: UFix64) { + self.balance = balance + } + + pub fun withdraw(amount: UFix64): @Vault { + self.balance = self.balance - amount + return <-create Vault(balance: amount) + } + + pub fun deposit(from: @Vault) { + self.balance = self.balance + from.balance + destroy from + } + } + + pub fun createEmptyVault(): @Bar.Vault { + return <- create Bar.Vault(balance: 0.0) + } + + pub fun createVault(balance: UFix64): @Bar.Vault { + return <- create Bar.Vault(balance: balance) + } + } + `) + + // Deploy Bar + + deployVault := DeploymentTransaction("Bar", bar) + err := runtime.ExecuteTransaction( + Script{ + Source: deployVault, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Deploy Attacker + + deployAttacker := DeploymentTransaction("Foo", attacker) + + err = runtime.ExecuteTransaction( + Script{ + Source: deployAttacker, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Attack + + attackTransaction := []byte(fmt.Sprintf(` + import Foo from %[1]s + import Bar from %[1]s + + transaction { + prepare(acc: AuthAccount) { + acc.save(<- Bar.createVault(balance: 100.0), to: /storage/vault)! + var vault = acc.borrow<&Bar.Vault>(from: /storage/vault)! + var flow <- vault.withdraw(amount: 42.0) + + var doubled <- Foo.doubler(<-flow) + log(doubled.balance) + destroy doubled + } + }`, + signerAccount.HexWithPrefix(), + )) + + err = runtime.ExecuteTransaction( + Script{ + Source: attackTransaction, + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) +} diff --git a/runtime/sema/check_assignment.go b/runtime/sema/check_assignment.go index 9d38a3294c..f18b330605 100644 --- a/runtime/sema/check_assignment.go +++ b/runtime/sema/check_assignment.go @@ -109,6 +109,18 @@ func (checker *Checker) checkAssignment( ResourceInvalidationKindMoveDefinite, ) + // Track nested resource moves. + // Even though this is needed only for second value transfers, it is added here because: + // 1) The second value transfers are checked as assignments, + // so the info needed (value's type etc.) is only available here. + // Adding it here covers second value transfers. + // 2) Having it in assignment would cover all cases, even the ones that are statically rejected by the checker. + // So this would also act as a defensive check for all other cases. + valueIsResource := valueType != nil && valueType.IsResourceType() + if valueIsResource { + checker.elaborateNestedResourceMoveExpression(value) + } + return } @@ -156,12 +168,6 @@ func (checker *Checker) visitAssignmentValueType( targetExpression ast.Expression, ) (targetType Type) { - inAssignment := checker.inAssignment - checker.inAssignment = true - defer func() { - checker.inAssignment = inAssignment - }() - // Check the target is valid (e.g. identifier expression, // indexing expression, or member access expression) diff --git a/runtime/sema/check_interface_declaration.go b/runtime/sema/check_interface_declaration.go index 2b2818fd69..4aa29e1d4e 100644 --- a/runtime/sema/check_interface_declaration.go +++ b/runtime/sema/check_interface_declaration.go @@ -32,6 +32,12 @@ import ( // through `declareInterfaceMembers`. func (checker *Checker) VisitInterfaceDeclaration(declaration *ast.InterfaceDeclaration) (_ struct{}) { + wasInInterface := checker.inInterface + checker.inInterface = true + defer func() { + checker.inInterface = wasInInterface + }() + const kind = ContainerKindInterface interfaceType := checker.Elaboration.InterfaceDeclarationType(declaration) diff --git a/runtime/sema/check_invocation_expression.go b/runtime/sema/check_invocation_expression.go index a1ffe69b42..3cd0122e64 100644 --- a/runtime/sema/check_invocation_expression.go +++ b/runtime/sema/check_invocation_expression.go @@ -69,6 +69,9 @@ func (checker *Checker) checkInvocationExpression(invocationExpression *ast.Invo invokedExpression := invocationExpression.InvokedExpression expressionType := checker.VisitExpression(invokedExpression, nil) + // `inInvocation` should be reset before visiting arguments + checker.inInvocation = false + // Get the member from the invoked value // based on the use of optional chaining syntax diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index 61cbf5b52f..588c18b4ef 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -282,8 +282,7 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedT // // This would result in a bound method for a resource, which is invalid. - if !checker.inAssignment && - !checker.inInvocation && + if !checker.inInvocation && member.DeclarationKind == common.DeclarationKindFunction && !accessedType.IsInvalidType() && accessedType.IsResourceType() { diff --git a/runtime/sema/checker.go b/runtime/sema/checker.go index e584939483..cee5eb9c66 100644 --- a/runtime/sema/checker.go +++ b/runtime/sema/checker.go @@ -110,8 +110,8 @@ type Checker struct { errors []error functionActivations *FunctionActivations inCondition bool + inInterface bool allowSelfResourceFieldInvalidation bool - inAssignment bool inInvocation bool inCreate bool isChecked bool @@ -1497,6 +1497,20 @@ func (checker *Checker) recordResourceInvalidation( return nil } + // Invalidations in interface functions are not allowed, + // except for temporary moves + if invalidationKind != ResourceInvalidationKindMoveTemporary && + checker.inCondition && + checker.inInterface { + + checker.report(&InvalidInterfaceConditionResourceInvalidationError{ + Range: ast.NewRangeFromPositioned( + checker.memoryGauge, + expression, + ), + }) + } + reportInvalidNestedMove := func() { checker.report( &InvalidNestedResourceMoveError{ diff --git a/runtime/sema/errors.go b/runtime/sema/errors.go index ac53910145..23adc045d5 100644 --- a/runtime/sema/errors.go +++ b/runtime/sema/errors.go @@ -2050,6 +2050,23 @@ func (e *InvalidNestedResourceMoveError) Error() string { return "cannot move nested resource" } +// InvalidInterfaceConditionResourceInvalidationError + +type InvalidInterfaceConditionResourceInvalidationError struct { + ast.Range +} + +var _ SemanticError = &InvalidInterfaceConditionResourceInvalidationError{} +var _ errors.UserError = &InvalidInterfaceConditionResourceInvalidationError{} + +func (*InvalidInterfaceConditionResourceInvalidationError) isSemanticError() {} + +func (*InvalidInterfaceConditionResourceInvalidationError) IsUserError() {} + +func (e *InvalidInterfaceConditionResourceInvalidationError) Error() string { + return "cannot invalidate resource in interface condition" +} + // InvalidResourceAnnotationError type InvalidResourceAnnotationError struct { diff --git a/runtime/tests/checker/composite_test.go b/runtime/tests/checker/composite_test.go index 0698938f93..b2fbec0675 100644 --- a/runtime/tests/checker/composite_test.go +++ b/runtime/tests/checker/composite_test.go @@ -1373,15 +1373,30 @@ func TestCheckInvalidCompositeFunctionAssignment(t *testing.T) { ), ) - errs := RequireCheckerErrors(t, err, 2) + if kind == common.CompositeKindResource { + errs := RequireCheckerErrors(t, err, 3) - require.IsType(t, &sema.AssignmentToConstantMemberError{}, errs[0]) - assert.Equal(t, - "foo", - errs[0].(*sema.AssignmentToConstantMemberError).Name, - ) + require.IsType(t, &sema.ResourceMethodBindingError{}, errs[0]) - assert.IsType(t, &sema.TypeMismatchError{}, errs[1]) + require.IsType(t, &sema.AssignmentToConstantMemberError{}, errs[1]) + assert.Equal(t, + "foo", + errs[1].(*sema.AssignmentToConstantMemberError).Name, + ) + + assert.IsType(t, &sema.TypeMismatchError{}, errs[2]) + + } else { + errs := RequireCheckerErrors(t, err, 2) + + require.IsType(t, &sema.AssignmentToConstantMemberError{}, errs[0]) + assert.Equal(t, + "foo", + errs[0].(*sema.AssignmentToConstantMemberError).Name, + ) + + assert.IsType(t, &sema.TypeMismatchError{}, errs[1]) + } }) } } diff --git a/runtime/tests/checker/resources_test.go b/runtime/tests/checker/resources_test.go index 65f79780fc..c12b0b4cf2 100644 --- a/runtime/tests/checker/resources_test.go +++ b/runtime/tests/checker/resources_test.go @@ -9358,3 +9358,437 @@ func TestCheckInvalidResourceDestructionInFunction(t *testing.T) { assert.IsType(t, &sema.ResourceCapturingError{}, errs[0]) } + +func TestCheckInvalidationInCondition(t *testing.T) { + + t.Parallel() + + testKind := func(kind ast.ConditionKind) { + t.Run(kind.Name(), func(t *testing.T) { + + t.Parallel() + + t.Run("global function", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, fmt.Sprintf( + ` + resource R {} + + fun drop(_ r: @R): Bool { + destroy r + return true + } + + fun test(_ r: @R) { + %s { + drop(<-r) + } + } + `, + kind.Keyword(), + )) + require.NoError(t, err) + }) + + t.Run("in composite", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, fmt.Sprintf( + ` + resource R {} + + fun drop(_ r: @R): Bool { + destroy r + return true + } + + struct S { + fun test(_ r: @R) { + %s { + drop(<-r) + } + } + } + `, + kind.Keyword(), + )) + require.NoError(t, err) + }) + + t.Run("in interface, definite invalidation", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, fmt.Sprintf( + ` + resource R {} + + fun drop(_ r: @R): Bool { + destroy r + return true + } + + struct interface S { + fun test(_ r: @R) { + %s { + drop(<-r) + } + } + } + `, + kind.Keyword(), + )) + + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.InvalidInterfaceConditionResourceInvalidationError{}, errs[0]) + }) + + t.Run("in interface, temporary invalidation", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, fmt.Sprintf( + ` + resource R {} + + struct interface SI { + fun drop(_ r: @R) { + %s { + r.isInstance(Type<@R>()) + } + } + } + `, + kind.Keyword(), + )) + require.NoError(t, err) + }) + + t.Run("in nested type requirement, definite invalidation", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, fmt.Sprintf( + ` + resource R {} + + fun drop(_ r: @R): Bool { + destroy r + return true + } + + contract interface CI { + struct S { + fun test(_ r: @R) { + %s { + drop(<-r) + } + } + } + } + `, + kind.Keyword(), + )) + + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.InvalidInterfaceConditionResourceInvalidationError{}, errs[0]) + }) + + t.Run("in nested type requirement, temporary invalidation", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, fmt.Sprintf( + ` + resource R {} + + contract interface CI { + struct S { + fun drop(_ r: @R) { + %s { + r.isInstance(Type<@R>()) + } + } + } + } + `, + kind.Keyword(), + )) + require.NoError(t, err) + }) + + }) + } + + for kind := ast.ConditionKindUnknown + 1; int(kind) < ast.ConditionKindCount(); kind++ { + testKind(kind) + } +} + +func TestCheckBoundFunctionToResource(t *testing.T) { + + t.Parallel() + + t.Run("simple invalid", func(t *testing.T) { + t.Parallel() + + _, err := ParseAndCheck(t, ` + access(all) resource R { + access(all) fun sayHi() {} + } + + access(all) fun main() { + var r <- create R() + + let bypass = fun(x: ((): Void)): ((): Void) { + return x + } + + // This is forbidden (ResourceMethodBindingError): + // var f = r.sayHi + // Passing to a function invocation as an argument should also be forbidden + var f = bypass(r.sayHi) + + destroy r + } + `) + + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.ResourceMethodBindingError{}, errs[0]) + }) + + t.Run("nested invalid", func(t *testing.T) { + t.Parallel() + + _, err := ParseAndCheck(t, ` + access(all) resource R { + access(all) fun sayHi() {} + } + + access(all) fun main() { + var r <- create R() + + let bypass = fun(x: ((): Void)): ((): ((): Void)) { + return fun(): ((): Void) { + return x + } + } + + // This is forbidden (ResourceMethodBindingError): + // var f = r.sayHi + // Passing to a function invocation as an argument should also be forbidden + var f = bypass(r.sayHi)() + + destroy r + } + `) + + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.ResourceMethodBindingError{}, errs[0]) + }) + + t.Run("nested valid", func(t *testing.T) { + t.Parallel() + + _, err := ParseAndCheck(t, ` + access(all) resource R { + access(all) fun sayHi() {} + } + + access(all) fun main() { + var r <- create R() + + let bypass = fun(x: Void): ((): Void) { + return fun(): Void { + return x + } + } + + // It's okay to use in an argument, as long as it's another invocation. + bypass(r.sayHi())() + + destroy r + } + `) + + require.NoError(t, err) + }) + + t.Run("inside index expr", func(t *testing.T) { + t.Parallel() + + _, err := ParseAndCheck(t, ` + access(all) resource R { + access(all) fun sayHi() {} + } + + access(all) fun main() { + var r <- create R() + var array: [((): Void)] = [] + + let bypass = fun(x: ((): Void)): Int { + return 0 + } + + array[bypass(r.sayHi)]() + + destroy r + } + `) + + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.ResourceMethodBindingError{}, errs[0]) + }) + + t.Run("as index", func(t *testing.T) { + t.Parallel() + + _, err := ParseAndCheck(t, ` + access(all) resource R { + access(all) fun sayHi() {} + } + + access(all) fun main() { + var r <- create R() + var array: [((): Void)] = [] + + let bypass = fun(x: ((): Void)): Int { + return 0 + } + + array[r.sayHi]() + + destroy r + } + `) + + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + }) + + t.Run("function expression", func(t *testing.T) { + t.Parallel() + + _, err := ParseAndCheck(t, ` + access(all) resource R { + access(all) fun sayHi() {} + } + + access(all) fun main() { + var r <- create R() + var array: [((): Void)] = [] + + let bypass = fun(x: ((): Void)): Int { + return 0 + } + + var i = fun(): Int { + var f = r.sayHi + return 0 + }() + + destroy r + } + `) + + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.ResourceCapturingError{}, errs[0]) + }) + + t.Run("array expression", func(t *testing.T) { + t.Parallel() + + _, err := ParseAndCheck(t, ` + access(all) resource R { + access(all) fun sayHi() {} + } + + access(all) fun main() { + var r <- create R() + var array: [((): Void)] = [] + + let bypass = fun(x: ((): Void)): Int { + return 0 + } + + [r.sayHi, r.sayHi][0]() + + destroy r + } + `) + + require.NoError(t, err) + }) + +} + +func TestCheckBoundFunctionToResourceInAssignment(t *testing.T) { + + t.Parallel() + + t.Run("valid assignment", func(t *testing.T) { + t.Parallel() + + _, err := ParseAndCheck(t, ` + access(all) resource R { + pub(set) var f: ((): Void) + + init() { + self.f = fun() {} + } + + fun assignNewValue() { + self.f = fun() {} + } + } + + access(all) fun someFunc(_ f: ((): Void)): Int { + return 0 + } + + access(all) fun main() { + var r <- create R() + var arr: [Int] = [1] + + // Must not report an error + r.f = fun(): Void {} + + destroy r + } + `) + + require.NoError(t, err) + }) + + t.Run("simple invalid", func(t *testing.T) { + t.Parallel() + + _, err := ParseAndCheck(t, ` + access(all) resource R { + access(all) fun f() {} + } + + access(all) fun someFunc(_ f: ((): Void)): Int { + return 0 + } + + access(all) fun main() { + var r <- create R() + var arr: [Int] = [1] + + // Must report an error! + arr[someFunc(r.f)] = 2 + + destroy r + } + `) + + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.ResourceMethodBindingError{}, errs[0]) + }) +} diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index 4d810ef4e5..ba6069c2d7 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -2557,38 +2557,49 @@ func TestInterpretResourceDestroyedInPreCondition(t *testing.T) { t.Parallel() - inter := parseCheckAndInterpret(t, ` - resource interface I { - pub fun receiveResource(_ r: @Bar) { - pre { - destroyResource(<-r) + inter, err := parseCheckAndInterpretWithOptions( + t, + ` + resource interface I { + pub fun receiveResource(_ r: @Bar) { + pre { + destroyResource(<-r) + } } } - } - - fun destroyResource(_ r: @Bar): Bool { - destroy r - return true - } - resource Foo: I { - pub fun receiveResource(_ r: @Bar) { + fun destroyResource(_ r: @Bar): Bool { destroy r + return true } - } - resource Bar {} + resource Foo: I { + pub fun receiveResource(_ r: @Bar) { + destroy r + } + } - fun test() { - let foo <- create Foo() - let bar <- create Bar() + resource Bar {} - foo.receiveResource(<- bar) - destroy foo - } - `) + fun test() { + let foo <- create Foo() + let bar <- create Bar() - _, err := inter.Invoke("test") + foo.receiveResource(<- bar) + destroy foo + } + `, + ParseCheckAndInterpretOptions{ + HandleCheckerError: func(err error) { + errs := checker.RequireCheckerErrors(t, err, 1) + + require.IsType(t, &sema.InvalidInterfaceConditionResourceInvalidationError{}, errs[0]) + }, + }, + ) + require.NoError(t, err) + + _, err = inter.Invoke("test") RequireError(t, err) require.ErrorAs(t, err, &interpreter.InvalidatedResourceError{}) @@ -2943,41 +2954,418 @@ func TestInterpretInnerResourceMove(t *testing.T) { t.Parallel() - inter := parseCheckAndInterpret(t, ` - pub resource OuterResource { - pub var a: @InnerResource - pub var b: @InnerResource + t.Run("assignment", func(t *testing.T) { + t.Parallel() - init() { - self.a <- create InnerResource() - self.b <- create InnerResource() - } + inter := parseCheckAndInterpret(t, ` + pub resource OuterResource { + pub var a: @InnerResource + pub var b: @InnerResource + + init() { + self.a <- create InnerResource() + self.b <- create InnerResource() + } + + pub fun swap() { + self.a <-> self.b + } + + destroy() { + // Nested resource is moved here once + var a <- self.a - pub fun swap() { - self.a <-> self.b + // Nested resource is again moved here. This one should fail. + self.swap() + + destroy a + destroy self.b + } } - destroy() { - // Nested resource is moved here once - var a <- self.a + pub resource InnerResource {} - // Nested resource is again moved here. This one should fail. - self.swap() + pub fun main() { + let a <- create OuterResource() + destroy a + }`, + ) + + _, err := inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.UseBeforeInitializationError{}) + }) + t.Run("second transfer", func(t *testing.T) { + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + pub resource OuterResource { + pub var a: @InnerResource + pub var b: @InnerResource + + init() { + self.a <- create InnerResource() + self.b <- create InnerResource() + } + + pub fun swap() { + self.a <-> self.b + } + + destroy() { + var a <- create InnerResource() + + // Nested resource is moved here once + var temp <- a <- self.a + + // Nested resource is again moved here. This one should fail. + self.swap() + + destroy a + destroy temp + destroy self.b + } + } + + pub resource InnerResource {} + + pub fun main() { + let a <- create OuterResource() destroy a - destroy self.b + }`, + ) + + _, err := inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.UseBeforeInitializationError{}) + }) +} + +func TestInterpretSetMemberResourceLoss(t *testing.T) { + + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + access(all) resource R { + access(all) let id: String + init(id: String) { + self.id = id + } + } + access(all) fun putBack(_ rl: &ResourceLoser, _ r: @R) { + rl.inner <-! r + } + access(all) resource ResourceLoser { + pub(set) var inner: @R?; + init(toLose: @R) { + self.inner <- toLose + } + destroy() { + var ref = &self as &ResourceLoser; + // Let's move self.inner out + var a <- self.inner + // And force-write it back + putBack(ref, <- a!) + } + } + access(all) fun main(): Void { + var resource <- create R(id: "abc"); + var rl <- create ResourceLoser(toLose: <- resource); + destroy rl + }`, + ) + + _, err := inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.DestroyedResourceError{}) +} + +func TestInterpretValueTransferResourceLoss(t *testing.T) { + + t.Parallel() + + inter, getLogs, _ := parseCheckAndInterpretWithLogs(t, ` + access(all) resource R { + access(all) let id: String + init(_ id: String) { + log("Creating ".concat(id)) + self.id = id + } + destroy() { + log("Destroying ".concat(self.id)) + } + } + + access(all) struct IndexSwitcher { + access(self) var counter: Int + init() { + self.counter = 0 + } + access(all) fun callback(): Int { + self.counter = self.counter + 1 + if self.counter == 1 { + // Which key we want to be read? + // Let's point it to a non-existent key + return 123 + } else { + // Which key we want to be assigned to? + // We point it to 0 to overwrite the victim value + return 0 } } + } + + access(all) fun loseResource(victim: @R) { + var in <- create R("dummy resource") + var dict: @{Int: R} <- { 0: <- victim } + var indexSwitcher = IndexSwitcher() + + // this callback should only be evaluated once, rather than twice + var out <- dict[indexSwitcher.callback()] <- in + destroy out + destroy dict + } + + access(all) fun main(): Void { + var victim <- create R("victim resource") + loseResource(victim: <- victim) + } + `, + ) + + _, err := inter.Invoke("main") + require.NoError(t, err) - pub resource InnerResource {} + assert.Equal(t, + []string{ + `"Creating victim resource"`, + `"Creating dummy resource"`, + `"Destroying dummy resource"`, + `"Destroying victim resource"`, + }, + getLogs(), + ) - pub fun main() { - let a <- create OuterResource() - destroy a - }`, +} + +func TestInterpretVariableDeclarationEvaluationOrder(t *testing.T) { + + t.Parallel() + + inter, getLogs, err := parseCheckAndInterpretWithLogs(t, ` + // Necessary helper interface, + // as AnyResource does not provide a uuid field, + // and AnyResource must be used in collect + // to avoid the potential type confusion to be rejected + // by the defensive argument/parameter type check + + resource interface HasID { + fun getID(): UInt64 + } + + resource Foo: HasID { + fun getID(): UInt64 { + return self.uuid + } + } + + resource Bar: HasID { + fun getID(): UInt64 { + return self.uuid + } + } + + fun collect(_ collected: @{HasID}): @Foo { + log("collected") + log(collected.getID()) + log(collected.getType()) + + destroy <- collected + + return <- create Foo() + } + + fun main() { + var foo <- create Foo() + log("foo") + log(foo.uuid) + + var bar <- create Bar() + log("bar") + log(bar.uuid) + + if (true) { + // Check that the RHS is evaluated *before* the variable is declared + var bar <- foo <- collect(<-bar) + + destroy foo + destroy bar // new bar + } else { + destroy foo + destroy bar // original bar + } + } + `) + require.NoError(t, err) + + _, err = inter.Invoke("main") + require.NoError(t, err) + + assert.Equal(t, + []string{ + `"foo"`, + `1`, + `"bar"`, + `2`, + `"collected"`, + `2`, + "Type()", + }, + getLogs(), ) +} + +func TestInterpretIfLetElseBranchConfusion(t *testing.T) { + + t.Parallel() + + inter, _, err := parseCheckAndInterpretWithLogs(t, ` + pub resource Victim{} + pub fun main() { + var r: @Victim? <- nil + var r2: @Victim? <- create Victim() + if let dummy <- r <- r2 { + // unreachable token destroys to please checker + destroy dummy + destroy r + } else { + // Error: r2 is invalid here + + var ref = &r as &Victim? + var arr: @[Victim?]<- [<- r, <- r2] + destroy arr + } + } + `) + require.NoError(t, err) + + _, err = inter.Invoke("main") + RequireError(t, err) + require.ErrorAs(t, err, &interpreter.InvalidatedResourceError{}) +} + +func TestInterpretMovedResourceInOptionalBinding(t *testing.T) { + + t.Parallel() + + inter, _, err := parseCheckAndInterpretWithLogs(t, ` + access(all) resource R{} + + access(all) fun collect(copy2: @R?, _ arrRef: &[R]): @R { + arrRef.append(<- copy2!) + return <- create R() + } + + access(all) fun main() { + var victim: @R? <- create R() + var arr: @[R] <- [] + + // In the optional binding below, the 'victim' must be invalidated + // before evaluation of the collect() call + if let copy1 <- victim <- collect(copy2: <- victim, &arr as &[R]) { + arr.append(<- copy1) + } else { + destroy victim // Never executed + } + + destroy arr // This crashes + } + `) + require.NoError(t, err) + + _, err = inter.Invoke("main") + RequireError(t, err) + invalidResourceError := &interpreter.InvalidatedResourceError{} + require.ErrorAs(t, err, invalidResourceError) + + // Error must be thrown at `copy2: <- victim` + errorStartPos := invalidResourceError.LocationRange.StartPosition() + assert.Equal(t, 15, errorStartPos.Line) + assert.Equal(t, 56, errorStartPos.Column) +} + +func TestInterpretMovedResourceInSecondValue(t *testing.T) { + + t.Parallel() + + inter, _, err := parseCheckAndInterpretWithLogs(t, ` + access(all) resource R{} + + access(all) fun collect(copy2: @R?, _ arrRef: &[R]): @R { + arrRef.append(<- copy2!) + return <- create R() + } + + access(all) fun main() { + var victim: @R? <- create R() + var arr: @[R] <- [] + + // In the optional binding below, the 'victim' must be invalidated + // before evaluation of the collect() call + let copy1 <- victim <- collect(copy2: <- victim, &arr as &[R]) + + destroy copy1 + destroy arr + } + `) + require.NoError(t, err) + + _, err = inter.Invoke("main") + RequireError(t, err) + invalidResourceError := &interpreter.InvalidatedResourceError{} + require.ErrorAs(t, err, invalidResourceError) + + // Error must be thrown at `copy2: <- victim` + errorStartPos := invalidResourceError.LocationRange.StartPosition() + assert.Equal(t, 15, errorStartPos.Line) + assert.Equal(t, 53, errorStartPos.Column) +} + +func TestInterpretOptionalBindingElseBranch(t *testing.T) { + + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + access(all) resource Victim {} + + access(all) fun main() { + + var r: @Victim? <- nil + var r2: @Victim? <- create Victim() + + if let dummy <- r <- r2 { + // unreachable token destroys to please checker + destroy dummy + destroy r + } else { + // checker failed to notice that r2 is invalid here + var ref = &r as &Victim? + var arr: @[Victim?]<- [ + <- r, + <- r2 + ] + destroy arr + } + } + `) _, err := inter.Invoke("main") RequireError(t, err) - require.ErrorAs(t, err, &interpreter.UseBeforeInitializationError{}) + + var invalidatedResourceErr interpreter.InvalidatedResourceError + require.ErrorAs(t, err, &invalidatedResourceErr) + + // Error must be thrown at `<-r2` in the array literal + assert.Equal(t, 18, invalidatedResourceErr.StartPosition().Line) }