# Bug Finding report

## Exercise 1:

Some malicious actors can push too many values into the safe making it very hard for other users to call `take` method since it loops through all the `safes` array. Also, the `take` function is using `transfer` method to transfer funds which depend on fixed gas and can be avoided. https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

## Exercise 2:

It will be impossible to satisfy the following condition at some point in `buy` method making it impossible to buy further objects.

> require(msg.value * (1 + objectBought[msg.sender]) == basePrice);

`basePrice` is 1 ether. Let's say at some stage, objectBought[msg.sender] = 2, then msg.value should be 0.333333...

## Exercise 3:

`lastChoiceHead` is a state variable and can be viewed outside from blockchain making it very simple for an attacker to guess the variable. Also, the `guess` function is using `transfer` method to transfer funds that depend on fixed gas and can be avoided. https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

## Exercise 4:

Reentrancy attack possible on `redeem` method causing it to empty the contract. It should use the check-effect-interactions pattern to avoid the issue.

## Exercise 5:

To stop `partyB` from taking funds because of the timeout, `partyA` needs to call `resolve` method passing `_chooseHead` and `_randomNumber` as the argument in the transaction. `partyB` can take advantage of this by frontrunning the transaction paying a higher gas fee and calling `guess` with the correct value. `commitmentA` is state variable so can be read from outside. `partyB` can also front-run `partyA` guess call.

## Exercise 6:

This contract is using `int` data type instead of `uint`. `int` can also hold negative values. So, even if a user doesn't hold any token, he can transfer tokens to other addresses turning the sender balance to zero increasing the receipient balance. This can result in infinite supply.

## Exercise 7:

When `totalSupply` is zero, the first user which buys the token can't sell all the tokens if any further transaction hasn't taken place. Let's say immediately after deployment of the contract, the user buys the token of 3 Wei and gets 3 tokens. Then, that user won't be able to sell all the 3 tokens.

## Exercise 8:

In the `withdraw` and `closeAccount` method, coffer's slot(`coffer.slots[_slot]`) is not set to zero. So, the user can once create a coffer slot and deposit ether and then close the account getting all the ether. If the user again creates a coffer slot, then the user can again withdraw ether without depositing any funds.

## Exercise 9:

User won't be able to deposit the funds which is equal to balance of contract because of following line:

> uint toAdd = (scalingFactor * msg.value) / (address(this).balance - msg.value)

The denominator will result in zero and the above expression will revert.

## Exercise 10:

In `deposit` function, funds sent won't be deposited in the smart contract but are sent to the owner's account.

> owner.send(baseDeposit);

Hence, the smart contract won't be having enough ether in some cases to send the ether to side A and side B resulting in a revert.

## Exercise 11:

It is possible to produce the same `ID` for different names, surnames and nonces due to the usage of `abi.encodePacked`.

> abi.encodePacked("aa","bb") is "aabb"
> abi.encodePacked("a","abb") is "aabb"

## Exercise 12:

If the user transfers a token to his address, the balance of the user will increase causing an infinite supply of tokens without the need of buying the tokens.

## Exercise 13:

`numberOfLosers` is being set to `winners.length` in `findWinners` function. Due to this, the following line will error out in `distribute` function since the denominator is zero. (Am not sure about this bug)

> payable(winners[i]).send(totalBalance / (winners.length - numberOfLosers));

## Exercise 14:

This contract assumes that funds can only be deposited to this contract using the deposit method and it is a multiple of 1. But it is possible to deposit the funds to this contract by specifying it as the recipient of self-destruct of another contract making it impossible to satisfy the required condition of `withdrawAll` method.

## Exercise 15:

The struct `Round` contains `rewards` and `isAllowed` mapping containing the address allowed to withdraw the rewards. The owner can clear the rounds. On deleting the `rounds` array, mapping is not deleted making older addresses withdraw the rewards again as they are stored into mapping.