Farseen0/scar-pairs huggingface dataset:
LOW severity: `Agreement::removeAccounts` can leave chain with zero accounts. **Description:** When creating or modifying chains via `Agreement::addChains`, `Agreement::addOrSetChains`, the `Agreement::_validateChains` function enforces that each chain must have at least one account:

However, `Agreement::removeAccounts` does not check whether removal would leave zero...

Vulnerable code:
function _validateChains(Chain[] memory _chains) internal {
    for (uint256 i = 0; i < _chains.length; i++) {
        // ...
        if (_chains[i].accounts.length == 0) { //@audit does not allow chain with zero accounts
            revert Agreement__ZeroAccountsForChainId(_chains[i].caip2ChainId);
        }
        // ...
    }
}

function removeAccounts(string memory _caip2ChainId, string[] memory _accountAddresses) external onlyOwner {
    if (!_chainExists(_caip2ChainId)) {
        revert Agreement__ChainNotFoundByCaip2Id(_caip2ChainId);
    }
    // @audit No check for remaining accounts after removal
    for (uint256 i = 0; i < _accountAddresses.length; i++) {
        uint256 accountIndex = _findAccountIndex(_caip2ChainId, _accountAddresses[i]);
        emit AccountRemoved(_caip2ChainId, _accountAddresses[i]);

        uint256 lastAccountId = accounts[_caip2ChainId].length - 1;
        accounts[_caip2ChainId][accountIndex] = accounts[_caip2ChainId][lastAccountId];
        accounts[_caip2ChainId].pop();
    }
}

function test_POC_removeAllAccountsFromChain() public {
        AgreementDetails memory detailsBefore = agreement.getDetails();
        assertEq(detailsBefore.chains[0].accounts.length, 1);

        string memory chainId = detailsBefore.chains[0].caip2ChainId;
        string memory accountAddr = detailsBefore.chains[0].accounts[0].accountAddress;

        // Remove the account
        string[] memory toRemove = new string[](1);
        toRemove[0] = accountAddr;

        vm.prank(owner);
        agreement.removeAccounts(chainId, toRemove);

        // Verify chain now has zero accounts
        AgreementDetails memory detailsAfter = agreement.getDetails();
        assertEq(detailsAfter.chains[0].accounts.length, 0);  // @audit chain now has zero accounts

    }
LOW severity: Optional `signature` field specified in legal agreement but not implemented in `Chain` struct. **Description:** The SEAL Whitehat Safe Harbor Agreement legal document specifies an optional `signature` field within the `Chain` struct that is not implemented in the smart contract code.

>Legal Agreement Reference (Section 1.1(c)(ii)):
"D. optionally, the signature for such account, which may...

Vulnerable code:
struct Chain {
    // The address to which recovered assets will be sent.
    string assetRecoveryAddress;
    // The accounts in scope for the agreement.
    Account[] accounts;
    // The CAIP-2 chain ID.
    string caip2ChainId;
    // @audit Missing: signature field as specified in agreement Section 1.1(c)(ii)(D)
}
LOW severity: Actual minting fee can differ from predicted fee. **Description:** `IPDerivativeAgent::registerDerivativeViaAgent` relies on `predictMintingLicenseFee(...)` to determine `tokenAmount`, transfers that amount from the caller, and approves the Royalty Module for exactly that amount. However, the fee ultimately paid during `registerDerivative(...)`...

Vulnerable code:
// Handle token payment if required
        if (currencyToken != address(0) && tokenAmount > 0) {
            IERC20 token = IERC20(currencyToken);

            // Transfer tokens from licensee to this contract
            token.safeTransferFrom(msg.sender, address(this), tokenAmount);

            // Increase allowance for RoyaltyModule to pull tokens during registerDerivative
+           token.safeIncreaseAllowance(ROYALTY_MODULE, maxMintingFee);
-           token.safeIncreaseAllowance(ROYALTY_MODULE, tokenAmount);
        }

        // ...

        // Clean up any remaining allowance for RoyaltyModule
        if (currencyToken != address(0) && tokenAmount > 0) {
            IERC20 token = IERC20(currencyToken);
            uint256 remainingAllowance = token.allowance(address(this), ROYALTY_MODULE);
            if (remainingAllowance > 0) {
+               token.safeTransfer(msg.sender, token.balanceOf(address(this)));
                token.forceApprove(ROYALTY_MODULE, 0);
            }
        }
HIGH severity: `DepositManager::_refundEntryFee` doesn't deduct referral rewards allowing users to join then leave games to drain tokens via inflated referral rewards they aren't entitled to. **Description:** `DepositManager::_payEntryFee` increments the referral rewards for the user's referrer:

But `DepositManager::_refundEntryFee` doesn't deduct referral rewards when a user leaves the game and their fee is refunded.

**Impact:** Malicious users can intentionally join then leave...

Vulnerable code:
referralRewards[gameId][Registry(registry).referrers(player)] += pool.ticketPrice * REFERRER_FEE;
HIGH severity: Attacker can drain all tokens from cancelled game since `SessionManager::refundCancelledGame` doesn't validate caller actually joined the game. **Description:** Attacker can drain all tokens from cancelled game since `SessionManager::refundCancelledGame` doesn't validate caller actually joined the game.

**Impact:** Any cancelled game can be completely drained of tokens by a permissionless attacker.

**Proof of Concept:** Add test to...

Vulnerable code:
function test_attackerDrainsCancelledGame() public {
    // create game
    _createGame();

    // contestant joins
    vm.startPrank(contestants[0]);
    TestUSDC(usdc).approve(address(sessionManager), 10 ether);
    sessionManager.joinGame(1);

    // game is cancelled
    vm.stopPrank();
    sessionManager.cancelGame(1);
    vm.warp(block.timestamp + (type(uint256).max - block.timestamp));

    // attacker who never joined gets a refund they don't deserve
    address attacker = address(0x1337);
    vm.startPrank(attacker);
    sessionManager.refundCancelledGame(1);
    vm.stopPrank();

    // attacker can repeat this using different addresses
    // which all get a refund even though they never joined
    // the game, until the tokens have been totally drained

    // attacker has drained all the tokens
    assertEq(TestUSDC(usdc).balanceOf(attacker), 10 ether);
    assertEq(TestUSDC(usdc).balanceOf(address(sessionManager)), 0 ether);
    (,,,, uint256 totalCollectedAmount, address token, bool feesPaid) = sessionManager.gamePools(1);
    assertEq(totalCollectedAmount, 0 ether);
    assertEq(token, usdc);
    assertEq(feesPaid, false);

    // user who actually joined game can't get refund as
    // tokens have been drained
    vm.expectRevert(); // NotEnoughFunds(0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f, 0)]
    vm.startPrank(contestants[0]);
    sessionManager.refundCancelledGame(1);
    vm.stopPrank();
}
HIGH severity: Impossible for user to get refund after re-joining a rescheduled game which is subsequently cancelled. **Description:** Impossible for user to get refund after re-joining a rescheduled game which is subsequently cancelled.

**Impact:** The user's fee for joining the game is permanently locked inside the immutable `SessionManager` contract.

**Proof of Concept:** Add the PoC to...

Vulnerable code:
function test_gameRescheduled_Leave_JoinAgain_GameCancelled_UserRefundReverts() public {
    // create a game
    uint256 timeAfterRescheduling = sessionManager.minimumRescheduleTime();
    _createGame();
    uint256 rescheduleDelta = type(uint256).max - sessionManager.getStartTime(1);

    // user joins the game
    uint256 gameId = 1;
    uint256 gameFee = 10 ether;
    address user = contestants[0];
    uint256 startTime = sessionManager.getStartTime(gameId);
    vm.startPrank(user);
    TestUSDC(usdc).approve(address(sessionManager), gameFee);
    sessionManager.joinGame(gameId);
    vm.stopPrank();

    // game gets rescheduled
    sessionManager.rescheduleGame(1, startTime + rescheduleDelta);
    vm.warp(block.timestamp + timeAfterRescheduling);

    // user leaves rescheduled game
    vm.prank(user);
    sessionManager.leaveRescheduledGame(gameId);

    // user got refunded the game fee
    assertEq(TestUSDC(usdc).balanceOf(user), gameFee);
    assertEq(TestUSDC(usdc).balanceOf(address(sessionManager)), 0 ether);

    // user decides to re-join the game
    vm.startPrank(user);
    TestUSDC(usdc).approve(address(sessionManager), gameFee);
    sessionManager.joinGame(gameId);
    vm.stopPrank();

    // user decides to leave again; impossible
    vm.expectRevert(); // AlreadyRefunded(0xd52E4d00E363cB91d9051fBFDC80c292a1da630B, 1)]
    vm.prank(user);
    sessionManager.leaveRescheduledGame(gameId);

    // game is cancelled
    sessionManager.cancelGame(gameId);

    // impossible for user to get a refund!
    vm.expectRevert(); // AlreadyRefunded(0xd52E4d00E363cB91d9051fBFDC80c292a1da630B, 1)]
    vm.prank(user);
    sessionManager.refundCancelledGame(gameId);

    // user's game fee is permanently stuck in the session manager contract!
    assertEq(TestUSDC(usdc).balanceOf(user), 0);
    assertEq(TestUSDC(usdc).balanceOf(address(sessionManager)), gameFee);
}

// reset user refunded status when joining the game; this allows
// users to get refunded if they rejoin a game which later gets cancelled
if(hasRefunded[gameId][player]) hasRefunded[gameId][player] = false;
HIGH severity: `DepositManager::getRewards` always includes `REFERRER_FEE` resulting in 2 percent of every games' rewards not being distributed to winners when there were no referrers. **Description:** `DepositManager::getRewards` always includes `REFERRER_FEE` when calculating the percentage of `totalCollectedAmount` available to distribute to winners:

**Impact:** If no referral rewards were accrued for a game, this calculation results in the game rewards being less than they...

Vulnerable code:
function getRewards(uint256 gameId) public view returns (uint256) {
    return gamePools[gameId].totalCollectedAmount
        * (BASIS_POINTS - (gamePools[gameId].creatorFee + gamePools[gameId].protocolFee + REFERRER_FEE)) / BASIS_POINTS;
}
HIGH severity: Impossible to claim rewards when ranked rewards or number of winners are not set, resulting in permanently locked tokens once game has concluded. **Description:** `FixedRanksReward::setRankedRewards` enforces that ranked rewards can only be set when the game is in the `Created` state:

The same is also true for `ProportionalToXPReward::setNumberOfWinners`.

But `SessionManager::startAndRevealGameQuestion` will happily start the game without.....

Vulnerable code:
function setRankedRewards(uint256 sessionId, uint256[] calldata _rankedRewards) external {
        require(sessionManager.getSessionState(sessionId) == SessionState.Created, NotCreated(sessionId));

function test_setRankedRewardsNotCalled_gameStarted_gameConcludes_cantClaimRewards() public {
    _createGame();

    _startGame();
    _revealQuestion();
    _warpToEndTime();
    sessionManager.endGame(1);
    _concludeGame();

    vm.expectRevert(); // RankedRewardsNotSet(1)
    vm.prank(contestants[0]);
    sessionManager.claimRewards(1, 0);
}
HIGH severity: Impossible to claim rewards when `XPTiers` are not set, resulting in permanently locked tokens once game has concluded. **Description:** `DefaultSession::setXPTiers` enforces that XP tiers  can only be set when the game is in the Created state:

But `SessionManager::startAndRevealGameQuestion`  will start the game without XP Tier being set, and the game will progress all the way to the final Concluded state. This...

Vulnerable code:
function setXPTiers(uint256 gameId, uint256[] memory _xpTiers) external {
        require(
            msg.sender == SessionManager(sessionManager).getCreator(gameId),
            NotGameCreator(SessionManager(sessionManager).getCreator(gameId), msg.sender)
        );
        require(_xpTiers.length >= 2, ArrayLengthMismatch());
        require(SessionManager(sessionManager).getSessionState(gameId) == SessionState.Created, GameNotCreated(gameId)); <--------
        xpTiers[gameId] = _xpTiers;
        emit XpTiersUpdated(gameId, _xpTiers);
    }

function getResult(uint256 gameId, uint256 questionId, address user) public view returns (Result memory) {
        SessionManager sessionManager = SessionManager(revealedQuestions[questionId].sessionManager);
        uint256 score = getScore(questionId, user);
        address sessionStrategy = sessionManager.getSessionStrategy(gameId);
        uint256[] memory xpTiers = ISessionStrategy(sessionStrategy).getXPTiers(gameId); <-------
        if (score > 0) {
            return
                Result({xp: xpTiers[0] / 2 + score * xpTiers[0] / 2 / 10000, time: _getReactionTime(questionId, user)});
        } else {
            return Result({xp: xpTiers[1], time: _getReactionTime(questionId, user)});
        }
    }

function assertResults(
        uint256 sessionId,
        string calldata resultCid,
        address[] calldata proposedWinners,
        uint256[] calldata totalXPs, <-----
        uint256[] calldata totalTimes
HIGH severity: If multiple users call `DefaultSession::assertResults` all but the first caller lose their bonds. **Description:** The `assertResults` is a permissionless function that allow anyone to assert a result for a `gameId` (sessionId):

Users that call this function have to pay a usdc bond of 250 dollars, see [minimum bonds value](https://docs.uma.xyz/resources/approved-collateral-types).

The...

Vulnerable code:
function assertResults(
        uint256 sessionId,
        string calldata resultCid,
        address[] calldata proposedWinners,
        uint256[] calldata totalXPs,
        uint256[] calldata totalTimes
    ) external returns (bytes32 assertionId) {
        require(SessionManager(sessionManager).getSessionState(sessionId) == SessionState.Ended, GameNotEnded());
        return assertDataFor(
            sessionId, resultCid, resolutionGitRepoAtCommitHash, proposedWinners, totalXPs, totalTimes, msg.sender
        );
    }

function assertDataFor(
        uint256 sessionId,
        string calldata resultCid,
        string memory resolutionGitRepoAtCommitHash,
        address[] calldata winners,
        uint256[] calldata totalXPs,
        uint256[] calldata totalTimes,
        address asserter
    ) internal returns (bytes32 assertionId) {
        asserter = asserter == address(0) ? msg.sender : asserter;
        uint256 bond = optimisticOracle.getMinimumBond(address(usdc)); /
        usdc.safeTransferFrom(msg.sender, address(this), bond);  <--------
        usdc.forceApprove(address(optimisticOracle), bond); <-------
}

function recordResults(uint256 sessionId, bytes32 assertionId) public {

        require(SessionManager(sessionManager).getSessionState(sessionId) == SessionState.Ended, GameNotEnded());
        require(
            sessionId == assertions[assertionId].sessionId,
            SessionIdMismatch(sessionId, assertions[assertionId].sessionId)
        );
        require(assertions[assertionId].resolved, AssertionNotInitialized(assertionId));
        require(winners[sessionId].length == 0, WinnersAlreadyRecorded(sessionId));
        ...

        winners[sessionId] = assertion.winners;
    }
MEDIUM severity: User can join after the first question is revealed to gain an advantage over other users. **Description:** Users can join a game while the game is ongoing:

`SessionManager::startAndRevealGameQuestion` both moves the game to the `Ongoing` state and reveals the first question.

**Impact:** A user can get an unfair advantage over others by always waiting for the first question to be...

Vulnerable code:
function joinGame(uint256 _gameId) external {
        require(
            games[_gameId].state == SessionState.Created || games[_gameId].state == SessionState.Ongoing,
            InvalidGameState(SessionState.Created, games[_gameId].state)
        );
    }

function joinGame(uint256 _gameId) external {
-      require(
-            games[_gameId].state == SessionState.Created || games[_gameId].state == SessionState.Ongoing,
-            InvalidGameState(SessionState.Created, games[_gameId].state)
-        );
+      require(
+            games[_gameId].state == SessionState.Created,
+            InvalidGameState(SessionState.Created, games[_gameId].state)
+        );
MEDIUM severity: `MajorityChoicePrompt`, `SPBinaryPrompt` and `TriviaChoicePrompt` will not work correctly when used with different instances of `SessionManager`. **Description:** `MajorityChoicePrompt` is supposed to support multiple instances of `SessionManager`, however every instance of `SessionManager` starts with `QuestionManager::nextQuestionId = 0`.

This is problematic as `MajorityChoicePrompt::revealReaction` does this:

When a user plays...

Vulnerable code:
Reaction storage r = reactions[_questionId][_user];
require(!r.baseReaction.reactions[_questionId][_user], AnswerAlreadyRevealed(_user, _gameId, _questionId));
MEDIUM severity: Incorrect `recordResult` recorded for each question in `recordResults`. **Description:** When an assertion is resolved the UMA oracle makes a call back to `DefaultSession::assertionResolvedCallback` if the assertion was truthful, it calls `recordResults`:

As you can see the `recordResults` is calling the `recordResult` function for the specific strategies:

The...

Vulnerable code:
function recordResults(uint256 sessionId, bytes32 assertionId) public {
        ...
        uint256[] memory questionIds = SessionManager(sessionManager).getQuestionsForGame(sessionId);

        for (uint256 i = 0; i < assertion.winners.length; ++i) {
            address winner = assertion.winners[i]; //@audit how many winners could be?
            for (uint256 j = 0; j < questionIds.length; ++j) {
                (, address promptStrategy) = SessionManager(sessionManager).questionCommitment(questionIds[j]);
                IPromptStrategy(promptStrategy).recordResult(
                    questionIds[j], winner, assertion.totalXPs[i], assertion.totalTimes[i]
                ); <------
            }
          ...
        }

        winners[sessionId] = assertion.winners;
    }

function recordResult(uint256 questionId, address player, uint256 xp, uint256 time) external {
        address sessionStrategy = SessionManager(revealedQuestions[questionId].sessionManager).getSessionStrategy(
            revealedQuestions[questionId].gameId
        );
        require(sessionStrategy == msg.sender, OnlySessionStrategy(sessionStrategy, msg.sender));

        results[questionId][player] = Result({xp: xp, time: time}); <------
    }
MEDIUM severity: If zero xp is earned by all users, once game has concluded `SessionManager::claimRewards` panic reverts due to division by zero but game also can't be cancelled resulting in locked tokens. **Description:** `ProportionalToXPReward::getReward` divides by `totalXP`, but if none of the users have earned XP, this will panic revert due to division by zero:

`DefaultSession::setXPTiers` does not enforce non-zero xp tiers, it only enforces that at least two tiers must exist:

**Impact:** If.....

Vulnerable code:
uint256 userXP;
uint256 totalXP;
for (uint256 i; i < winners.length; ++i) {
    (, uint256 xp,) = sessionStrategy.userResult(sessionId, winners[i]);
    if (i == position) {
        userXP = xp;
    }
    totalXP += xp;
}
reward = userXP * prizePool / totalXP;

function setXPTiers(uint256 gameId, uint256[] calldata _xpTiers) external {
    require(
        msg.sender == SessionManager(sessionManager).getCreator(gameId),
        NotGameCreator(SessionManager(sessionManager).getCreator(gameId), msg.sender)
    );
    require(_xpTiers.length >= 2, ArrayLengthMismatch());
    require(xpTiers[gameId].length == 0, XpTiersAlreadySet(gameId));
    require(SessionManager(sessionManager).getSessionState(gameId) == SessionState.Created, GameNotCreated(gameId));
    xpTiers[gameId] = _xpTiers;
    emit XpTiersSet(gameId, _xpTiers);
}
MEDIUM severity: No validation on `reactionDeadline` allows multiple griefing scenarios. **Description:** When a creator creates a game they send an array of bytes32  `promptHash`  variables associated with the questions:

These `promptHash` are then reveled in the `_revealPrompt` function when creator call `startAndRevealGameQuestion` or `revealGameQuestion`, the `_revealPrompt` is...

Vulnerable code:
function createGame(
        uint256 _startTime,
        uint256 _endTime,
        uint256 _ticketPrice,
        uint256 _creatorFee,
        address _token,
        address _creatorFeeReceiver,
        bytes32[] memory _promptHashes, <-----
        address[] memory _promptStrategies,
        address _sessionStrategy,
        address _rewardStrategy,
        bool _verificationRequired
    ) external returns (uint256 gameId) {...}

function revealQuestion(bytes memory question, uint256 questionId) external {
        Prompt memory q = abi.decode(question, (Prompt));  <------
        require(registry.engageProtocols(msg.sender), InvalidSessionManager(msg.sender));
        require(q.sessionManager == msg.sender, OnlySessionManager(q.sessionManager, msg.sender));
        (, address promptStrategy) = SessionManager(q.sessionManager).questionCommitment(questionId);
        require(promptStrategy == address(this), InvalidPromptCall(questionId, promptStrategy));
        revealedQuestions[questionId] = q;
        revealedAt[questionId] = block.timestamp;
    }

struct Prompt {
        address sessionManager;
        uint256 gameId;
        string questionText;
        uint256 reactionDeadline;  <-----
        string finalizedAnswer;
        string[] media; <------
        string[] choices; <------
    }
MEDIUM severity: Game creator can call `TriviaChoicePrompt::revealSolutions` before the `reactionDeadline` or end of game, griefing players from submitting answers while still retaining player entry fees. **Description:** `TriviaChoicePrompt::revealSolutions` calls `_revealSolutions` which doesn't validate that `reactionDeadline` has expired. This causes `TriviaChoicePrompt::commitReaction` to revert when users try to submit answers:

**Impact:** A malicious game creator can immediately reveal...

Vulnerable code:
function commitReaction(uint256 _gameId, uint256 _questionId, bytes32 _commit, address _user) external {
        require(revealedAt[_questionId] != 0, QuestionNotRevealed(_questionId));
        require(
            revealedQuestions[_questionId].sessionManager == msg.sender,
            OnlySessionManager(revealedQuestions[_questionId].sessionManager, msg.sender)
        );
        require(solutionRevealedAt[_questionId] == 0, SolutionAlreadyRevealed(_questionId)); <------
        require(
            revealedAt[_questionId] + revealedQuestions[_questionId].reactionDeadline > block.timestamp,
            ReactionDeadlinePassed(_user, _questionId)
        );
        Reaction storage r = reactions[_questionId][_user];
        require(r.baseReaction.timestamp == 0, AnswerAlreadyCommitted(_user, _gameId, _questionId));

        r.baseReaction.commit = _commit;
        r.baseReaction.timestamp = block.timestamp;

        emit AnswerCommitted(_gameId, _questionId, _user, _commit);
    }

function test_solution_as_soon_as_reveledQuestion() public {
        triviaChoice.setRevealedQuestions();
        triviaChoice.revealSolutions(
            1, Solarray.uint256s(1), Solarray.bytess(abi.encode(uint16(1))), Solarray.uint256s(1234)
        );

        vm.expectRevert(abi.encodeWithSelector(TriviaChoicePrompt.SolutionAlreadyRevealed.selector, 1));
        triviaChoice.commitReaction(1, 1, keccak256(abi.encode(uint16(1))), address(this));
    }