code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L109 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L127

Vulnerability details

Impact

Privileged addresses with unlimited access to functions can create huge damage, including a loss of funds.

Proof of Concept

There are several ways such a change can occur, below we will explain one potential way:

1) Alice accesses a phishing page which claims to be biconomi-management.com (this page is compatible with Biconomy and offers a wallet management UI) 2) Alice wants to change the entryPoint or owner of her wallet. 3) The phishing page executes Alice's desired transaction, however, the within the calldata, a malicious owner or entryPoint is given. 4) As soon as the transaction succeeded, the malicious party will drain Alice's wallet

The same issue applies for adding new modules, however, since the adding logic is different, this would need another mitigation step than mentioned here. I won't create a new issue for the modules but this should be kept in mind.

** The same issue can also happen if a known website is being hacked.

Tools Used

VSCode, practical experience as (almost) victim with phishing & calldata manipulation

Recommended Mitigation Steps

Consider implementing a two-step authentification:

1)

setFutureOwner(address _owner) external onlyOwner {
          require(_owner != address(0), "Zero address");
          futureOwner = _owner;
      }

2)

claimFutureOwner(address _owner) external onlyOwner{
       require(futureOwner == _owner);
      owner = _owner;
      futureOwner = address(0);
}

Obviously, these issues cannot be completely prevented, however, using the provided example will make phishing attacks much harder.

*** We acknowledge the publicly known issues, however, it was not thought about a solution or a phishing attack.

c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2023-01-biconomy-findings/issues/438

c4-sponsor commented 1 year ago

livingrockrises marked the issue as sponsor acknowledged