Summary

The provided changeOwner() function, when analyzed in combination with the above testing code, has critical security flaws that can lead to unauthorized ownership transfers, causing potential financial and access risks to the contract.

Vulnerability Details

Lack of Access Control:

• The changeOwner() function does not restrict who can call it. Anyone (including malicious actors) can invoke this function and change the ownership to themselves or another arbitrary address.

• In the above test (testChangeOwner_AccessControl()), the test simulates an attack where user2 calls changeOwner() and becomes the new owner. This is a security breach because any user, not just the contract owner, can take control of the contract.

• Immediate Ownership Takeover:

  • Since the function can be called by anyone, a malicious actor can instantly change the ownership of the contract to their address without any permission from the current owner.

  • Once the ownership is changed, the attacker can execute privileged functions, like withdrawFunds(), to drain the contract balance.

Impact

• Ownership Takeover:

  • Any external user can become the contract owner by calling the changeOwner() function, leading to the complete loss of control for the legitimate owner.

  • Once ownership is taken over, the attacker can call any restricted functions intended only for the owner, such as withdrawFunds(), to steal the contract’s ether or manipulate other critical owner-only operations.

• Financial Loss:

  • In the provided test code, user1 buys boxes, and the funds are accumulated in the contract. Once user2 (who was not the original owner) calls changeOwner(), they are able to call withdrawFunds() and steal 0.3 ether.

  • In real-world scenarios, the contract could hold significantly larger balances, making this an avenue for large-scale theft of funds.

• Trust and Integrity Issues:

  • If ownership can be arbitrarily taken by anyone, it undermines trust in the system. Participants may lose faith in the contract's security, leading to reputational damage and reluctance to interact with it.

Tools Used

Foundry Testing

This test simulates a scenario where user1 buys two boxes and user2 becomes the new owner of the contract.

• It verifies that user2 successfully changes ownership and can withdraw the funds collected from the buyBox() calls.

• The test ensures that the funds are correctly transferred to user2 and the contract's ownership/access control mechanisms work as expected.

Recommendations

Add a check to ensure that only the current owner can call the changeOwner() function. This can be done using a modifier:

• This ensures that only the legitimate owner can change ownership.