_Follow along with this video:_ --- ### Continuing with selectWinner We've only just started with the `selectWinner` function and we've already found another issue. Let's keep going and see if we can find more. ```js function selectWinner() external { require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over"); require(players.length >= 4, "PuppyRaffle: Need at least 4 players"); uint256 winnerIndex = uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length; address winner = players[winnerIndex]; // @Audit: Why the calculationi for totalAmountCollected, why not address(this).balance? uint256 totalAmountCollected = players.length * entranceFee; // @Audit:80% prizePool, 20% fee. Is this correct? Arithmatic may lead to precision loss uint256 prizePool = (totalAmountCollected * 80) / 100; uint256 fee = (totalAmountCollected * 20) / 100; // @Audit: Total fees the owner should be able to collect. Why the casting? Overflow. totalFees = totalFees + uint64(fee); ... ``` Assessing the function snippet above I notice a few things that may be worth noting in our `notes.md` and/or by leaving in-line notes like shown. ```js totalFees = totalFees + uint64(fee); ``` This line in particular sets my alarm bells off. My experience tells me that this is at risk of `integer overflow`. This is a bit of a classic issue, as newer versions of Solidity (>=0.8.0) are protected from it. Head back to [**sc-exploits-minimized**](https://github.com/Cyfrin/sc-exploits-minimized) and let's have a closer look at how this works. Navigating to `src/arithmetic/OverflowAndUnderflow.sol` we can see a simple example of how this works. ```js contract Overflow { uint8 public count; // uint8 has a max value of 255, so if we add 1 to 255, we get 0 if it's unchecked! // Versions prior to 0.8 of solidity also have this issue function increment(uint8 amount) public { unchecked { count = count + amount; } } } ``` `unchecked` is a keyword in later versions of Solidity, this is being used to tell the compiler not to check for things like overflow. In earlier versions of Solidity (prior to 0.8.0) there were no checks by default. ### Overflow Remix Example We've provide a [**Remix example**](https://remix.ethereum.org/#url=https://github.com/Cyfrin/sc-exploits-minimized/blob/main/src/arithmetic/OverflowAndUnderflow.sol&lang=en&optimize=false&runs=200&evmVersion=null&version=soljson-v0.8.20+commit.a1b79de6.js) to experiment with and get a sense of things. By compiling and deploying our `Overflow.sol` contract, we should be met with this: ::image{src='/security-section-4/29-exploit-integer-overflow/overflow1.png' style='width: 75%; height: auto;'} The max value of a uint8 is 255. Our `count` variable starts at 0, so let's just pick a number to start with, say 200. ::image{src='/security-section-4/29-exploit-integer-overflow/overflow2.png' style='width: 75%; height: auto;'} Calling increment updates our `count` variable. No problem so far. Now let's add 60 to our number. `count` should total 260, but what do you think we'll get? ::image{src='/security-section-4/29-exploit-integer-overflow/overflow3.png' style='width: 75%; height: auto;'} We get 4! This is because our integer is hitting the cap of 255, and then wrapping back to 0. > **Note:** This true for ints and uints in all versions of Solidity **prior to** 0.8.0. > > In Solidity versions 0.8.0+ `unchecked` is required to expose this vulnerability. Uints and ints are `checked` by default. If a max is surpassed in these versions, the transaction will revert. The situation is the same in circumstances of `underflow`. An integer will wrap to the max value if reduced past it's limit. You can practice this with our remix example as well. ```js contract Underflow { uint8 public count; // uint8 has a min value of 0, but if we subtract 1 from 0, we get 255 if it's unchecked! // Versions prior to 0.8 of solidity also have this issue function decrement() public { unchecked { count--; } } } ``` ### Precision Loss The last vulnerability outlined in this repo is `precision loss`. ```js contract PrecisionLoss { uint256 public moneyToSplitUp = 225; uint256 public users = 4; // This function will return 56, but we want it to return 56.25 function shareMoney() public view returns (uint256) { return moneyToSplitUp / users; } } ``` ::image{src='/security-section-4/29-exploit-integer-overflow/overflow4.png' style='width: 75%; height: auto;'} At its root, this is because Solidity doesn't support float point numbers. Any time we're performing a division operation, we need to be aware of this potential loss of precision. ### Wrap Up A Proof of Concept/Code for this vulnerability should be pretty straightforward, so I won't be walking through one, but I challenge you to write one yourself. If you get stuck - you can check out the [**audit-data**](https://github.com/Cyfrin/4-puppy-raffle-audit/tree/audit-data) branch of the Puppy Raffle Repo for guidance. **_Don't Cheat!_** Let's keep going!