Lines of code
<https://github.com/GenerationSoftware/pt-v5-draw-auction/blob/f1c6d14a1772d6609de1870f8713fb79977d51c1/src/RngRelayAuction.sol#L241-L243&gt;

Vulnerability details

Impact

The RngRelayAuction contract deployed on each chain has a rngComplete method that is supposed to be called by the relayer in order to close/complete a prize draw.

However this method doesn’t have any access control and can therefore be called by anyone with arbitrary random numbers or sequence ids. And arbitrary random number can be supplied to ensure that the caller always wins a prize. And the maximum _sequenceId value can be supplied to perpetually brick the relay auction contract.

Proof of Concept

This vulnerability exists due to lack of access control:

  function rngComplete(
    uint256 _randomNumber,
    uint256 _rngCompletedAt,
    address _rewardRecipient,
    uint32 _sequenceId,
    AuctionResult calldata _rngAuctionResult
  ) external returns (bytes32) {
    if (_sequenceHasCompleted(_sequenceId)) revert SequenceAlreadyCompleted();
    uint64 _auctionElapsedSeconds = uint64(block.timestamp < _rngCompletedAt ? 0 : block.timestamp - _rngCompletedAt);
    if (_auctionElapsedSeconds > (_auctionDurationSeconds-1)) revert AuctionExpired();
    // Calculate the reward fraction and set the draw auction results
    UD2x18 rewardFraction = _fractionalReward(_auctionElapsedSeconds);
    _auctionResults.rewardFraction = rewardFraction;
    _auctionResults.recipient = _rewardRecipient;
    _lastSequenceId = _sequenceId;

    AuctionResult[] memory auctionResults = new AuctionResult[](2);
    auctionResults[0] = _rngAuctionResult;
    auctionResults[1] = AuctionResult({
      rewardFraction: rewardFraction,
      recipient: _rewardRecipient
    });

Supplying an arbitrary _randomNumber value is clearly an issue since the number is now not random and doesn’t originate from an on-chain random number generator. As stated above, a malicious user can ensure they win every round by supplying a random number that always places them in the winning zone.

In terms of bricking the contract, this can be achieved due to the call to _sequenceHasCompleted on the first line of the above method:

  function _sequenceHasCompleted(uint32 _sequenceId) internal view returns (bool) {
    return _lastSequenceId >= _sequenceId;
  }

This method checks that the supplied _sequenceId is greater than the last sequence id. Due to the lack of access control a malicious user can call rngComplete with _sequenceId = type(uint32).max to permanently brick the contract going forwards.

Tools Used

Manual review

Recommended Mitigation Steps

The rngComplete method should only be able to be called by the auction relayer on the respective chain. Below is a suggested diff:

diff --git a/src/RngRelayAuction.sol b/src/RngRelayAuction.sol
index 8085169..56d794a 100644
--- a/src/RngRelayAuction.sol
+++ b/src/RngRelayAuction.sol
@@ -135,6 +135,7 @@ contract RngRelayAuction is IRngAuctionRelayListener, IAuction {
     uint32 _sequenceId,
     AuctionResult calldata _rngAuctionResult
   ) external returns (bytes32) {
+    require(msg.sender == rngAuctionRelayer, "Not authorized");
     if (_sequenceHasCompleted(_sequenceId)) revert SequenceAlreadyCompleted();
     uint64 _auctionElapsedSeconds = uint64(block.timestamp < _rngCompletedAt ? 0 : block.timestamp - _rngCompletedAt);
     if (_auctionElapsedSeconds > (_auctionDurationSeconds-1)) revert AuctionExpired();

Assessed type

Access Control

---

The text was updated successfully, but these errors were encountered:

All reactions