sherlock-audit / 2022-11-nounsdao-judging

4 stars 0 forks source link

ctf_sec - Does not check if the Stream.sol#payment token matches the payer#payment token. #23

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ctf_sec

medium

Does not check if the Stream.sol#payment token matches the payer#payment token.

Summary

Does not check if the Stream.sol#payment token is the same as the payer#payment token.

Vulnerability Detail

I want to quote from the doc:

Creating a DAO proposal with Streamer

First the user (in our case the DAO's web UI) calls predictStreamAddress to get the future stream contract address. Then a proposal can be composed with the following two transactions:

StreamFactory.createStream(...), where the new stream's address should match predictStreamAddress Payer.sendOrRegisterDebt(...), where the payment recipient is the address from predictStreamAddress Once executed, the Token Buyer and Payer contracts work together to fund the new stream asynchronously, and once funded the stream's recipient can withdraw their streamed funds.

The Payer address has a payment token address.

https://github.com/nounsDAO/token-buyer/blob/33171cfbdb52cf9d5bd6520f1baec0b64b2f7168/src/Payer.sol#L39

/// @notice The ERC20 token used to pay users
IERC20 public immutable paymentToken;

the Stream contract has a payment token as well.

https://github.com/sherlock-audit/2022-11-nounsdao/blob/main/src/StreamFactory.sol#L202

stream = streamImplementation.cloneDeterministic(
    encodeData(payer, recipient, tokenAmount, tokenAddress, startTime, stopTime),
    salt(
        msg.sender, payer, recipient, tokenAmount, tokenAddress, startTime, stopTime, nonce
    )
);

and

/**
 * @notice Get this stream's ERC20 token.
 * @dev Uses clone-with-immutable-args to read the value from the contract's code region rather than state to save gas.
 */
function token() public pure returns (IERC20) {
    return IERC20(_getArgAddress(92));
}

these two payment token address should match, otherwise recipient cannot get paid properly.

Impact

if the Stream.sol#payment token does not match the payer#payment token, recipient get paid in wrong token.

Code Snippet

https://github.com/sherlock-audit/2022-11-nounsdao/blob/main/src/Stream.sol#L96-L104

https://github.com/sherlock-audit/2022-11-nounsdao/blob/main/src/StreamFactory.sol#L183-L214

Tool used

Manual Review

Recommendation

Check if the Stream.sol#payment token matches the payer#payment token before creating the stream contract.

adding the check

 address paymentAddress = IPayer(payer).paymentAddress();
 require(tokenAddress == paymentAddress, 'payment token mismatch');

before

  stream = streamImplementation.cloneDeterministic(
      encodeData(payer, recipient, tokenAmount, tokenAddress, startTime, stopTime),
      salt(
          msg.sender, payer, recipient, tokenAmount, tokenAddress, startTime, stopTime, nonce
      )
  );
  IStream(stream).initialize();
davidbrai commented 1 year ago

again, the payer in can be any address, and in our use case, the DAO's timelock, not the Payer contract from token-buyer