Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Bahurum - Incorrect contractAddress in tx receipt #176

Closed
github-actions bot opened this issue Feb 20, 2023 · 1 comment
Closed

Bahurum - Incorrect contractAddress in tx receipt #176

github-actions bot opened this issue Feb 20, 2023 · 1 comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Reward A payout will be made for this issue Specification An issue related to the specification (low severity)

Comments

@github-actions
Copy link

github-actions bot commented Feb 20, 2023

Bahurum

medium

Incorrect contractAddress in tx receipt

Summary

In state_processor.go function applyTransaction the ContractAddress is generated using tx.Nonce(), which is always 0 for deposit tx. This causes the contractAddress in the tx recipe to be always the same for successive deposit tx from the same sender.

Vulnerability Detail

In state_processor.go tx.Nonce() is used for generating the ContractAddress to be put in the tx receipt if the tx creates a contract. For deposit tx the nonce is always zero, so the contractAddress in the recipe will always be the same for the deposits from the same sender.
The same issue is present in execution.go

Impact

The tx recipe is incorrect as it shows the same contract being created multiple times, while contracts are created without their address being included in the receipts. This will cause issues to external applications querying the receipts to track contract creations. As an example, this already causes issues in the block exporer.

Code Snippet

https://github.com/sherlock-audit/2023-01-optimism/blob/main/op-geth/core/state_processor.go#L129

https://github.com/sherlock-audit/2023-01-optimism/blob/main/op-geth/cmd/evm/internal/t8ntool/execution.go#L218

Tool used

Manual Review

Recommendation

Use the sender account nonce from the state instead of the tx nonce.

 if msg.To() == nil {
-     receipt.ContractAddress = crypto.CreateAddress(evm.TxContext.Origin, tx.Nonce())
+     receipt.ContractAddress = crypto.CreateAddress(evm.TxContext.Origin, statedb.GetNonce(msg.From()))
 }

Duplicate of #204

@rcstanciu
Copy link
Contributor

Comment from Optimism


Description: Contract addresses incorrect in transaction receipts

Reason: The contract address field is non-consensus. As a result, this issue only appears when querying these broken deposits via RPC. The contract address in the state is correct. Reducing to low because it is not a consensus issue.

Action: Fix the contract address generation

@sherlock-admin sherlock-admin added the Non-Reward This issue will not receive a payout label Feb 21, 2023
@rcstanciu rcstanciu added Specification An issue related to the specification (low severity) Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Reward A payout will be made for this issue and removed Non-Reward This issue will not receive a payout labels Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Reward A payout will be made for this issue Specification An issue related to the specification (low severity)
Projects
None yet
Development

No branches or pull requests

2 participants