EthSecurity
5.22K subscribers
112 photos
20 files
764 links
Download Telegram
Watch out for arbitrary NFT approvals inside of loops! 👇

If you attempt to call the "setApprovalForAll" function on an AxieInfinity NFT for an address that already has approval, it will revert.
@EthSecurity1
The CoreCollection.withdraw function uses payableToken.transferFrom(address(this), msg.sender, amount) to transfer tokens from the CoreCollection contract to the msg.sender ( who is the owner of the contract). The usage of transferFrom can result in serious issues. In fact, many ERC20 always require that in transferFrom allowance[from][msg.sender] >= amount, so in this case the call to the withdraw function will revert as the allowance[CoreCollection][CoreCollection] == 0 and therefore the funds cannot ben withdrawn and will be locked forever in the contract.

Recommendation : replace transferFrom with transfer @EthSecurity1
4
tldr of tornado governance hack
1. hacker makes a proposal that executes code from a contract
2. users vote for the proposal since contract code looks good, proposal passes
3. hacker self-destructs contract and deploys malicious one in same address
4. 2nd contract is executed

so hacker got voters to vote a proposal, and, after the proposal passed, they changed the code for it and executed their malicious proposal, giving themselves full control of the DAO and draining the tokens held there.

another resource:

https://twitter.com/samczsun/status/1660012956632104960?s=21

@EthSecurity1
😁1
Storage collision because of lack of EIP1967 could cause conflicts and override sensible variables
Proof of Concept

contract CoreProxy is Ownable {
address private immutable _implement;

When you implement proxies, logic and implementation share the same storage layout. In order to avoid storage conflicts EIP1967 was proposed.(https://eips.ethereum.org/EIPS/eip-1967) The idea is to set proxy variables at fixed positions (like impl and admin ).

For example, according to the standard, the slot for for logic address should be

0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc (obtained as bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1) ).

In this case, for example, as you inherits from Ownable the variable _owner is at the first slot and can be overwritten in the implementation. There is a table at OZ site:https://docs.openzeppelin.com/upgrades-plugins/1.x/proxies @EthSecurity1
🔥1
Anyone holding funds in Tornado Cash Nova must withdraw the funds ! The attacker can simply upgrade the contract (takes 7 days tho to execute) on Gnosis Chain (is managed by governance) and drain the ETH funds. For how to do it, see here

https://t.co/LXoG9cEMpn
@EthSecurity1
👍1
lack of expiration timestamp check and slippage control. impact:
The transaction can be pending in mempool for a long and the trading activity is very time senstive. Without deadline check, the trade transaction can be executed in a long time after the user submit the transaction, at that time, the trade can be done in a sub-optimal price, which harms user's position.

The deadline check ensure that the transaction can be executed on time and the expired transaction revert. the second point is the slippage control:

require(amountAOptimal >= amountAMin, 'UniswapV2Router: INSUFFICIENT_A_AMOUNT');
and
require(amountBOptimal >= amountBMin, 'UniswapV2Router: INSUFFICIENT_B_AMOUNT');

the slippage control the user can receive the least optimal amount of the token they want to trade.

In the current implementation, neither the deadline check nor the slippage control is in place when user deposit / withdraw / trade. @EthSecurity1
1🔥1
EthSecurity
lack of expiration timestamp check and slippage control. impact: The transaction can be pending in mempool for a long and the trading activity is very time senstive. Without deadline check, the trade transaction…
mitigation:
We recommend the protocol add deadline check and add slippage control. NOTE! You must Stop Using block.timestamp as Deadline in Swaps. the takeaway is this: protocols should let users who interact with AMMs set expiration deadlines. Without this, there's a risk of a serious loss of funds for anyone starting a swap, especially if there's no slippage parameter. @EthSecurity1
🔥3
This research introduces a novel pairs trading strategy based on copulas for cointegrated
pairs of cryptocurrencies: https://arxiv.org/pdf/2305.06961.pdf @EthSecurity1
New paper “Automated Market Making and Arbitrage Profits in the Presence of Fees” by jason_of_cs ciamac Tim_Roughgarden
@EthSecurity1
https://moallemi.com/ciamac/papers/lvr-fee-model-2023.pdf
2