Farseen0/scar-pairs huggingface dataset:
LOW severity: ETH_SIGN'd messages should include len(message) and full message, not hash. In both `_validateOrderValidityOracleClaim()` and `_validateAssetPriceClaim()`, we check the message that has been signed by the off-chain oracle by encoding the data as follows:

This does not follow the format laid out in EIP 191, which specifies that the message following the prefix should be...

Vulnerable code:
bytes32 claimHash = keccak256(claimEncoded);
bytes32 prefixedHash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", claimHash));

bytes32 claimLen = len(claimEncoded);
bytes32 prefixedHash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", claimLen, claimEncoded));
HIGH severity: AURA token will not be accounted for in `tokensIn`. When the controller sees a call to Aura's `getReward()` function, it uses the following logic to set `tokensOut` and `tokensIn`:

This sets the `tokensIn` to equal an array with all the `extraRewards` tokens, as well as the target contract's `rewardToken`.

However, if we examine the code itself,...

Vulnerable code:
function canCallGetReward(address target) internal view returns (bool, address[] memory, address[] memory) {
    uint256 rewardLength = IRewards(target).extraRewardsLength();
    address[] memory tokensIn = new address[](rewardLength + 1);
    for (uint256 i = 0; i < rewardLength; i++) {
        tokensIn[i] = IRewards(IRewards(target).extraRewards(i)).rewardToken();
    }
    tokensIn[rewardLength] = IRewards(target).rewardToken();
    return (true, tokensIn, new address[](0));
}

IDeposit(operator).rewardClaimed(pid, _account, reward);

function rewardClaimed(uint256 _pid, address _address, uint256 _amount) external returns(bool){
    address rewardContract = poolInfo[_pid].crvRewards;
    require(msg.sender == rewardContract || msg.sender == lockRewards, "!auth");

    //mint reward tokens
    ITokenMinter(minter).mint(_address,_amount);

    return true;
}
HIGH severity: Many 0x transformations ignore outputToken, returning unaccounted for tokens to account and risking liquidations. When we call `transformERC20()` on the 0x contract, the following inputs are included as arguments:

`TransformController.sol` assumes that `outputToken` will be the only token returned.

However, if we look at the implementation of the function on 0x, we see the following flow:
- all tokens are...

Vulnerable code:
function transformERC20(
    address inputToken,
    address outputToken,
    uint256 inputTokenAmount,
    uint256 minOutputTokenAmount,
    Transformation[] calldata transformations
) external payable returns (uint256 outputTokenAmount);

interface IERC20Token {
    function balanceOf(address) external returns(uint);
}

struct TransformData {
    // The tokens to transfer to the taker.
    IERC20Token[] tokens;
    // Amount of each token in `tokens` to transfer to the taker.
    // `uint(-1)` will transfer the entire balance.
    uint256[] amounts;
}
LOW severity: Aura's `withdraw()` and `redeem()` functions do not send any reward tokens. When the controller sees a call to Aura's `withdraw()` or `redeem()` function, it sets `tokensIn` to an array of the `asset` along with all reward tokens:

However, if we examing the code, we will see that no rewards are sent, so the `tokensIn` array could simply be set to...

Vulnerable code:
function canCallWithdrawAndRedeem(address target)
    internal
    view
    returns (bool, address[] memory, address[] memory)
{
    uint256 rewardLength = IRewards(target).extraRewardsLength();
    address[] memory tokensIn = new address[](rewardLength + 2);
    for (uint256 i = 0; i < rewardLength; i++) {
        tokensIn[i] = IRewards(IRewards(target).extraRewards(i)).rewardToken();
    }
    tokensIn[rewardLength] = IERC4626(target).asset();
    tokensIn[rewardLength + 1] = IRewards(target).rewardToken();

    address[] memory tokensOut = new address[](1);
    tokensOut[0] = target;
    return (true, tokensIn, tokensOut);
}

function _withdrawAndUnwrapTo(uint256 amount, address from, address receiver) internal updateReward(from) returns(bool){
    //also withdraw from linked rewards
    for(uint i=0; i < extraRewards.length; i++){
        IRewards(extraRewards[i]).withdraw(from, amount);
    }

    _totalSupply = _totalSupply.sub(amount);
    _balances[from] = _balances[from].sub(amount);

    //tell operator to withdraw from here directly to user
    IDeposit(operator).withdrawTo(pid,amount,receiver);
    emit Withdrawn(from, amount);

    emit Transfer(from, address(0), amount);

    return true;
}

function withdraw(address _account, uint256 amount)
    public
    updateReward(_account)
{
    require(msg.sender == address(deposits), "!authorized");
    //require(amount > 0, 'VirtualDepositRewardPool : Cannot withdraw 0');

    emit Withdrawn(_account, amount);
}
LOW severity: If ETH is used as an input and output token to 0x, it will always revert. When transaction data to `transformERC20()` is decoded, the following check is made:

The intention is that, if a token going in or out of our sentiment wallet is ETH, we return an empty array. This is because (a) Sentiment balances automatically account for ETH and (b) the address representing...

Vulnerable code:
(address tokenOut, address tokenIn) =
    abi.decode(data[4:], (address, address));

if (tokenIn == ETH) {
    tokensOut = new address[](1);
    tokensOut[0] = tokenOut;
    return (true, new address[](0), tokensOut);
}

if (tokenOut == ETH) {
    tokensIn = new address[](1);
    tokensIn[0] = tokenIn;
    return (true, tokensIn, new address[](0));
}

function testEthOutIfAlsoIn() public {
    ITransformERC20Feature.Transformation[] memory transformations = new ITransformERC20Feature.Transformation[](0);
    bytes memory data = abi.encodeWithSelector(
        ITransformERC20Feature.transformERC20.selector, ETH, ETH, 0, 0, transformations
    );

    (bool canCall, address[] memory tokensIn, address[] memory tokensOut) =
        controllerFacade.canCall(target, true, data);

    assert(tokensOut[0] == ETH);
}

if (tokenIn == ETH) {
+   if (tokenOut == ETH) return (true, new address[](0), new address[](0));
    tokensOut = new address[](1);
    tokensOut[0] = tokenOut;
    return (true, new address[](0), tokensOut);
}

// no need to check it here because it would already be caught above
if (tokenOut == ETH) {
    tokensIn = new address[](1);
    tokensIn[0] = tokenIn;
    return (true, tokensIn, new address[](0));
}
HIGH severity: Budget cap will not account for transfers of approved or permitted ERtokens. `ApprovalVotingModule.sol` contains a feature where the governor can input a `budgetToken` and a `budgetAmount`, and the module ensures that passed proposals cannot spend more than this amount of this token.

This check occurs in `_formatExecuteParams()`. In this function, we loop over all of the...

Vulnerable code:
for (n = 0; n < option.targets.length;) {
    // Shortcircuit if `budgetAmount` is exceeded
    if (settings.budgetAmount != 0) {
        if (settings.budgetToken == address(0)) {
            // If `budgetToken` is ETH and value is not zero, add msg value to `totalValue`
            if (option.values[n] != 0) totalValue += option.values[n];
        } else {
            // If `target` is `budgetToken` and calldata is not zero
            if (settings.budgetToken == option.targets[n]) {
                bytes memory data = option.calldatas[n];
                if (data.length != 0) {
                    uint256 amount;
                    // If it's a `transfer` or `transferFrom`, add `amount` to `totalValue`
                    if (bytes4(data) == IERC20.transfer.selector) {
                        assembly {
                            // Load the last 32 bytes of `data` into 'amount'
                            amount := mload(add(data, 0x44))
                        }
                    } else if (bytes4(data) == IERC20.transferFrom.selector) {
                        assembly {
                            // Load the last 32 bytes of `data` into 'amount'
                            amount := mload(add(data, 0x64))
                        }
                    }
                    if (amount != 0) totalValue += amount;
                }
            }
        }

        // Break loop if `budgetAmount` is exceeded
        if (totalValue > settings.budgetAmount) break;
    }

    unchecked {
        executeParams[executeParamsLength + n] =
            ExecuteParams(option.targets[n], option.values[n], option.calldatas[n]);

        ++n;
    }
}
MEDIUM severity: New proposals can be DOS'd by frontrunning. To create a new proposal using the `ApprovalVotingModule`, the Manager of the governor contract calls `proposeWithModule()`:

[OptimismGovernorV5.sol#L77-L104](https://github.com/voteagora/optimism-gov/blob/35f441738bd7864bd37949a40842486bc0ac51b0/src/OptimismGovernorV5.sol#L77-L104)

The...

Vulnerable code:
function proposeWithModule(VotingModule module, bytes memory proposalData, string memory description)
    public
    onlyManager
    returns (uint256)
{
    require(
        getVotes(_msgSender(), block.number - 1) >= proposalThreshold(),
        "Governor: proposer votes below proposal threshold"
    );

    uint256 proposalId = hashProposalWithModule(address(module), proposalData, keccak256(bytes(description)));

    ProposalCore storage proposal = _proposals[proposalId];
    require(proposal.voteStart.isUnset(), "Governor: proposal already exists");

    uint64 snapshot = block.number.toUint64() + votingDelay().toUint64();
    uint64 deadline = snapshot + votingPeriod().toUint64();

    proposal.voteStart.setDeadline(snapshot);
    proposal.voteEnd.setDeadline(deadline);
    proposal.votingModule = address(module);

    emit ProposalCreated(proposalId, _msgSender(), address(module), proposalData, snapshot, deadline, description);

    module.propose(proposalId, proposalData);

    return proposalId;
}

function hashProposalWithModule(address module, bytes memory proposalData, bytes32 descriptionHash)
    public
    view
    virtual
    returns (uint256)
{
    return uint256(keccak256(abi.encode(address(this), module, proposalData, descriptionHash)));
}

function propose(uint256 proposalId, bytes memory proposalData) external override {
    if (_proposals[proposalId].governor != address(0)) revert ExistingProposal();
    ...
}
MEDIUM severity: Any address can be passed as a VotingModule, which could lead to abuse. In `OptimismGovernorV5.sol`, the `proposeWithModule()` function has the following signature:

This allows the caller to input a `VotingModule` address, and that module will be used to:

- set up the proposal (`module.propose()`)
- return the execution data (`module._formatExecuteParams()`)
-...

Vulnerable code:
function proposeWithModule(VotingModule module, bytes memory proposalData, string memory description);
MEDIUM severity: Votes can be arbitrarily extended by Manager until they meet quorum. In `OptimismGovernorV5.sol`, new proposals are created with a `snapshot` (start time) and `deadline` (end time):

All significant parameters on a proposal are locked once the vote is underway:

- a proposal's `votingModule` is immutable and cannot be changed
- updates to `quorum` are saved...

Vulnerable code:
uint64 snapshot = block.number.toUint64() + votingDelay().toUint64();
uint64 deadline = snapshot + votingPeriod().toUint64();

proposal.voteStart.setDeadline(snapshot);
proposal.voteEnd.setDeadline(deadline);
proposal.votingModule = address(module);

function setProposalDeadline(uint256 proposalId, uint64 deadline) public onlyManager {
    _proposals[proposalId].voteEnd.setDeadline(deadline);
    emit ProposalDeadlineUpdated(proposalId, deadline);
}

function setProposalDeadline(uint256 proposalId, uint64 deadline) public onlyManager {
+   require(block.timestamp < _proposals[proposalId].voteStart.getDeadline());
    _proposals[proposalId].voteEnd.setDeadline(deadline);
    emit ProposalDeadlineUpdated(proposalId, deadline);
}
LOW severity: Quorum initialized to 0.03% instead of 30% due to overridden denominator. When `OptimismGovernorV5.sol` is initialized, the `quorumNumerator` is set to `30`:

This value is intended to represent a 30% quorum when the denominator is set to 100, which is the default value set by OpenZeppelin and is represented in...

Vulnerable code:
function initialize(IVotesUpgradeable _votingToken, address _manager) public initializer {
    __Governor_init("Optimism");
    __GovernorCountingSimple_init();
    __GovernorVotes_init(_votingToken);
    __GovernorVotesQuorumFraction_init({quorumNumeratorValue: 30});
    __GovernorSettings_init({initialVotingDelay: 6575, initialVotingPeriod: 46027, initialProposalThreshold: 0});

    manager = _manager;
}

function quorumDenominator() public view virtual returns (uint256) {
    return 100;
}

function quorumDenominator() public view virtual override returns (uint256) {
    // Configurable to 3 decimal points of percentage
    return 100_000;
}
MEDIUM severity: Bounty manager can set `maxRewardPerVote` to 1 after votes are locked, stealing free votes. When the amount to pay for a `claim` is calculated, we use the minimum value of `bias * rewardPerVote` and `bias * maxRewardPerVote`.

Since `maxRewardPerVote` is a value that managers can edit at any time, this allows them to effectively reduce the bounty payouts to ~0 after votes have already...

Vulnerable code:
// Compute the reward amount based on
// Reward / Total Votes.
amount = _bias.mulWad(rewardPerVote[_bountyId]);

// Compute the reward amount based on
// the max price to pay.
uint256 _amountWithMaxPrice = _bias.mulWad(bounty.maxRewardPerVote);

// Distribute the _min between the amount based on votes, and price.
amount = FixedPointMathLib.min(amount, _amountWithMaxPrice);
MEDIUM severity: `createBounty()` doesn't confirm that a `rewardToken` exists, which can be used to steal deterministic tokens. When a new bounty is created with the `createBounty()` method, we use the solady `SafeTransferLib` to transfer the token into the contract:

Digging into this function, we can see that, in the event that there is no return data, the function will always pass:

This means that a user can pass a...

Vulnerable code:
SafeTransferLib.safeTransferFrom(rewardToken, msg.sender, address(this), totalRewardAmount);

function safeTransferFrom(address token, address from, address to, uint256 amount) internal {
    /// @solidity memory-safe-assembly
    assembly {
        let m := mload(0x40) // Cache the free memory pointer.

        // Store the function selector of `transferFrom(address,address,uint256)`.
        mstore(0x00, 0x23b872dd)
        mstore(0x20, from) // Store the `from` argument.
        mstore(0x40, to) // Store the `to` argument.
        mstore(0x60, amount) // Store the `amount` argument.

        if iszero(
            and( // The arguments of `and` are evaluated from right to left.
                // Set success to whether the call reverted, if not we check it either
                // returned exactly 1 (can't just be non-zero data), or had no return data.
                or(eq(mload(0x00), 1), iszero(returndatasize())),
                call(gas(), token, 0, 0x1c, 0x64, 0x00, 0x20)
            )
        ) {
            // Store the function selector of `TransferFromFailed()`.
            mstore(0x00, 0x7939f424)
            // Revert with (offset, size).
            revert(0x1c, 0x04)
        }

        mstore(0x60, 0) // Restore the zero slot to zero.
        mstore(0x40, m) // Restore the free memory pointer.
    }
}

uint size;
assembly {
    size := extcodesize(rewardToken)
}
return size > 0;
MEDIUM severity: Upgrades are put into effect immediately during first period. When upgrades are made by a bounty manager by calling the `increaseBountyDuration()` function, they are stored in the `upgradeBountyQueue` mapping. The intention is that, when the next period is started, all upgrades will be put into effect and cleared out of the queue.

This is accomplished...

Vulnerable code:
function _updateBountyPeriod(uint256 bountyId) internal returns (uint256) {
    Period storage _activePeriod = activePeriod[bountyId];

    uint256 currentPeriod = getCurrentPeriod();

    if (_activePeriod.id == 0 && currentPeriod == _activePeriod.timestamp) {
        // Check if there is an upgrade in queue and update the bounty.
        _checkForUpgrade(bountyId);

        // Initialize reward per token.
        // Only for the first period, and if not already initialized.
        _updateRewardPerToken(bountyId, currentPeriod);
    }

    // Increase Period
    if (block.timestamp >= _activePeriod.timestamp + _WEEK) {
        // Checkpoint gauge to have up to date gauge weight.
        gaugeController.checkpoint_gauge(bounties[bountyId].gauge);

        // Check if there is an upgrade in queue and update the bounty.
        _checkForUpgrade(bountyId);

        // Roll to next period.
        _rollOverToNextPeriod(bountyId, currentPeriod);

        return currentPeriod;
    }

    return _activePeriod.timestamp;
}

function testZach__UpgradesInFirstPeriodAreImmediate() public {
    // Create Bounty throught the platform contract.
    uint256 id = _createCustomBounty();

    // Retrieve the Bounty with the Id.
    Platform.Bounty memory oldBounty = platform.getBounty(id);
    gaugeController.checkpoint_gauge(gauge);

    // Need to skip 2 week since the first period start one week after the bounty creation.
    skip(WEEK);
    gaugeController.checkpoint_gauge(gauge);

    // Claim the reward with a random account just to trigger roll over.
    vm.prank(address(0xBABA));
    platform.claim(id);

    rewardToken.mint(user, amount);
    rewardToken.approve(address(platform), amount);
    // Increase the number of periods.

    platform.increaseBountyDuration(id, 2, amount, 3e18);
    platform.updateBountyPeriod(id);

    Platform.Bounty memory bounty = platform.getBounty(id);
    assertEq(bounty.numberOfPeriods, 4);
}

- if (_activePeriod.id == 0 && currentPeriod == _activePeriod.timestamp) {
+ if (_activePeriod.id == 0 && currentPeriod == _activePeriod.timestamp && rewardPerVote[bountyId] == 0) {
    // Check if there is an upgrade in queue and update the bounty.
    _checkForUpgrade(bountyId);

    // Initialize reward per token.
    // Only for the first period, and if not already initialized.
    _updateRewardPerToken(bountyId, currentPeriod);
  }
MEDIUM severity: Users cannot claim bounty in final week of their lock. The `bias` used to determine how many votes a user is contributing to a given gauge is calculated as follows:

The check to short circuit and return `0` is too restrictive, and also results in returning `0` for the final (valid) week of a stake.

We can understand this intuitively by seeing that...

Vulnerable code:
function _getAddrBias(uint256 userSlope, uint256 endLockTime, uint256 currentPeriod)
    internal
    pure
    returns (uint256)
{
    if (currentPeriod + _WEEK >= endLockTime) return 0;
    return userSlope * (endLockTime - currentPeriod);
}

uint256 private constant _WEEK = 1 weeks;

    function _getAddrBias(uint256 userSlope, uint256 endLockTime, uint256 currentPeriod)
        internal
        pure
        returns (uint256)
    {
        if (currentPeriod + _WEEK >= endLockTime) return 0;
        return userSlope * (endLockTime - currentPeriod);
    }

    function getCurrentPeriod() public view returns (uint256) {
        return (block.timestamp / _WEEK) * _WEEK;
    }

    function testZach__AddrBiasFinalWeek() public {
        uint256 userSlope = 1; // should be 604_800 per week
        uint256 currentPeriod = getCurrentPeriod();
        console.log("Bias If Ends Current Period + 0: ", _getAddrBias(userSlope, currentPeriod, currentPeriod));
        console.log("Bias If Ends Current Period + 1: ", _getAddrBias(userSlope, currentPeriod + 1 weeks, currentPeriod));
        console.log("Bias If Ends Current Period + 2: ", _getAddrBias(userSlope, currentPeriod + 2 weeks, currentPeriod));
        console.log("Bias If Ends Current Period + 3: ", _getAddrBias(userSlope, currentPeriod + 3 weeks, currentPeriod));
    }

Logs:
  Bias If Ends Current Period + 0:  0
  Bias If Ends Current Period + 1:  0
  Bias If Ends Current Period + 2:  1209600
  Bias If Ends Current Period + 3:  1814400
MEDIUM severity: Race condition if bounty is extended in final period. When a manager calls `increaseBountyDuration()` to queue up a change in their bounty's parameters, there is a check that the bounty is still active:

This check reverts if the bounty is past the `endTimestamp`, but succeeds otherwise, because `getPeriodsLeft` is inclusive (ie it returns 1 in the...

Vulnerable code:
if (getPeriodsLeft(_bountyId) < 1) revert NO_PERIODS_LEFT();

if (block.timestamp >= _activePeriod.timestamp + _WEEK) {
            // Checkpoint gauge to have up to date gauge weight.
            gaugeController.checkpoint_gauge(bounties[bountyId].gauge);

            // Check if there is an upgrade in queue and update the bounty.
            _checkForUpgrade(bountyId);

            // Roll to next period.
            _rollOverToNextPeriod(bountyId, currentPeriod);

            return currentPeriod;
        }
LOW severity: Blacklisted addresses will count towards total slope if they vote before rollover. When we roll over to a new period, we calculate the `rewardPerVote` by taking the `rewardPerPeriod` and dividing by the total bias (votes) for the gauge.

In order to ensure these calculations are accurate, blacklisted addresses are not included in the total bias calculation. Instead, they are...

Vulnerable code:
uint256 gaugeBias = _getAdjustedBias(bounty.gauge, bounty.blacklist, currentPeriod);
rewardPerVote[bountyId] = rewardPerPeriod.mulDiv(_BASE_UNIT, gaugeBias);

gaugeBias = gaugeController.points_weight(gauge, period).bias;

for (uint256 i = 0; i < length;) {
    // Get the user slope.
    userSlope = gaugeController.vote_user_slopes(_addressesBlacklisted[i], gauge);
    _lastVote = gaugeController.last_user_vote(_addressesBlacklisted[i], gauge);
    if (period > _lastVote) {
        _bias = _getAddrBias(userSlope.slope, userSlope.end, period);
        gaugeBias -= _bias;
    }
    // Increment i.
    unchecked {
        ++i;
    }
}
LOW severity: Owner can steal funds from any user by abusing `setRecipientFor()` or `platformFee`. The owner has the ability to steal all bounty rewards from users. This can be accomplished in either of two ways:

1. The owner has access to a function called `setRecipientFor()`, which allows them to set the recipient for any address.

If this value is set to a malicious address, it can be...

Vulnerable code:
function setRecipientFor(address _for, address _recipient) external onlyOwner {
    recipient[_for] = _recipient;

    emit RecipientSet(_for, _recipient);
}
LOW severity: Manager can be set to address(0), which prevents bounty from being closed. When a bounty is completed, the `closeBounty()` function is called to send the funds back to the manager. `closeBounty()` uses the manager's address as a proxy for whether the bounty has already been closed. It checks for this value at the start of the function, and deletes it at the end of the...

Vulnerable code:
function closeBounty(uint256 bountyId) external nonReentrant {
    Bounty storage bounty = bounties[bountyId];
    if (bounty.manager == address(0)) revert ALREADY_CLOSED();

    ...

    delete bounties[bountyId].manager;
    emit BountyClosed(bountyId, leftOver);
}

if (rewardToken == address(0) || manager == address(0)) revert ZERO_ADDRESS();

function updateManager(uint256 bountyId, address newManager) external onlyManager(bountyId) {
    emit ManagerUpdated(bountyId, bounties[bountyId].manager = newManager);
}