Saddle Finance

This [economic attack](https://medium.com/@peter_4205/curve-vulnerability-report-a1d7630140ec) on the Curve contracts was discovered by Peter Zeitz. Since the Saddle Finance contracts are a Solidity implementation of the Curve contracts they are also vulnerable to the same attack.

Projects may rely on block timestamps for various purposes. However, it's important to realize that miners individually set the timestamp of a block, and attackers may be able to manipulate timestamps for their own purposes. If a smart contract relies on a timestamp, it must take this into account. The following functions which are used to determine the value of the A parameter use the block timestamp: * `SwapUtils._getAPrecise` * `SwapUtils.rampA` * `SwapUtils.stopRampA` The value of A is used to compute a large number of other crucial amounts in the system, including the values of D, the virtual price and the amount of tokens added/withdrawn. It is important to note that a malicious miner can change the value of `block.timestamp` with up to 900 seconds.

In function `Allowlist.setMultipliers`, duplicate addresses in the input arrays will cause overwrites of multipliers (in the case of human errors), during execution.

There is no check that constrains the owner of the `Allowlist` contract from setting the `multipliers`, `poolCaps` and `accountLimits` to such values which would result in a call to `getAllowedAmount` for some pool to be greater than `getPoolCap` for the same pool. Would this be acceptable? <br> Similarly the values of `multipliers` can be greater than the value of `DENOMINATOR`. Would this be acceptable?

Projects may rely on block timestamps for various purposes. However, it's important to realize that miners individually set the timestamp of a block, and attackers may be able to manipulate timestamps for their own purposes. If a smart contract relies on a timestamp, it must take this into account. The following functions/modifiers use the block timestamp: * `Swap.deadlineCheck` * `SwapUtils.calculateCurrentWithdrawFee` * `SwapUtils.updateUserWithdrawFee` * `SwapUtils._getAPrecise` * `SwapUtils.rampA` * `SwapUtils.stopRampA`

The accuracy of the return value of the `SwapUtils.getD(uint256[] memory xp, uint256 _A)` function could be affected by the loss of precision that occurs with the repeated divisions that occur on L241 (inside the `for`-loop), which are all performed before the multiplications in the subsequent iterations of the loop and the multiplication of `dP` with `numTokens` on L244 (after the `for`-loop). ``` L240: for (uint j = 0; j < numTokens; j++) { L241: dP = dP.mul(D).div(xp[j].mul(numTokens).add(1)); L242: } L243: prevD = D; L244: D = nA.mul(s).add(dP.mul(numTokens)).mul(D).div( L245: nA.sub(1).mul(D).add(numTokens.add(1).mul(dP))); ```

Smart contracts will often have `owner` variables to designate the person with special privileges to make modifications to the smart contract. The following instances were identified in this project: 1. The owner of the `Allowlist` contract can perform the following actions: * Set the multipliers for any addresses to any value, any number of times. * Set the account limits for any pool to any value, any number of times. * Set the pool cap for any pool to any value, any number of times. 1. The owner of the `LPToken` contract can mint any amount of tokens to any address. There is no cap. 1. The owner of the `Swap` contract can: * Set admin fee values and withdraw the admin fees at any point in time as many times as they want. * Set the swap fee values at any point in time as many times as they want. * Set the default withdrawal fee values at any point in time as many times as they want. * Set the guarded status of the deposits. If set to `false` the pool will allow deposits over the allowed limit per user and will allow the TVL to go over the pool cap.