OKX

In the contract `StakingRewards`, the role `rewardsDistribution` has authority over the functions shown in the diagram below.
Any compromise to the `rewardsDistribution` account may allow the hacker to take advantage of this authority and change the amount of rewards or the reward duration.
![](https://accelerator-tasks-prod.acc.corp.certik.com/009df8265b654454a8173f122fa7002e/diagrams/centralization_StakingRewards-StakingRewards-rewardsDistribution.svg)

In the contract `StakingRewardsFactory`, the role `_owner` has authority over the functions shown in the diagram below.
Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and change the amount of rewards or the reward duration for staking contracts.
![](https://accelerator-tasks-prod.acc.corp.certik.com/009df8265b654454a8173f122fa7002e/diagrams/centralization_StakingRewardsFactory-StakingRewardsFactory-_owner.svg)

In the contract `OKCSwapFactory`, the role `feeToSetter` has authority over the functions shown in the diagram below.
Any compromise to the `feeToSetter` account may allow the hacker to take advantage of this authority and change the amount of liquidity fees as well as change the fee recipient.
![](https://accelerator-tasks-prod.acc.corp.certik.com/009df8265b654454a8173f122fa7002e/diagrams/centralization_OKCSwapFactory-OKCSwapFactory-feeToSetter.svg)

In the contract `OKCSwapPair` the role `factory` has authority over the functions shown in the diagram below.
Any compromise to the `factory` account may allow the hacker to take advantage of this authority and initialize the incorrect tokens for the pair.
![](https://accelerator-tasks-prod.acc.corp.certik.com/009df8265b654454a8173f122fa7002e/diagrams/centralization_OKCSwapPair-OKCSwapPair-factory.svg)

When transferring deflationary ERC20 tokens, the input amount may not be equal to the received amount due to the charged transaction fee. For example, if a user sends 100 deflationary tokens (with a 10% transaction fee), only 90 tokens actually arrived to the contract. However, a failure to discount such fees may allow the same user to withdraw 100 tokens from the contract, which causes the contract to lose 10 tokens in such a transaction.

Reference:
https://thoreum-finance.medium.com/what-exploit-happened-today-for-gocerberus-and-garuda-also-for-lokum-ybear-piggy-caramelswap-3943ee23a39f

```solidity=90
        stakingToken.safeTransferFrom(msg.sender, address(this), amount);
```
- Transferring tokens by `amount`.

```solidity=85
        _balances[msg.sender] = _balances[msg.sender].add(amount);
```
- The `amount` is used for bookkeeping purposes without compensating for potential transfer fees.


---
```solidity=98
        stakingToken.safeTransferFrom(msg.sender, address(this), amount);
```
- Transferring tokens by `amount`.

```solidity=97
        _balances[msg.sender] = _balances[msg.sender].add(amount);
```
- The `amount` is used for bookkeeping purposes without compensating for potential transfer fees.


---
```solidity=45
            require(
                rewardsToken.transfer(stakingRewards, rewardAmount),
                'StakingRewardsFactory::notifyRewardAmount: transfer failed'
            );
```
- Transferring tokens by `rewardAmount`.

```solidity=49
            StakingRewards(stakingRewards).notifyRewardAmount(rewardAmount,rewardsDuration);
```
- The `rewardAmount` is used for bookkeeping purposes without compensating for potential transfer fees.

### Proof of Concept

The following unit test created using `forge` demonstrates the issue by performing the following:
1. A deflationary ERC20 token is created where when transferring tokens, only 90% of the inputted amount is transferred
2. The test mints 100 tokens to the contract and the contract stakes 100 tokens
3. The staking balance is compared with ERC20 balance of the staking contract
4. The test fails as expected as the staking balance is 100 while the ERC20 balance is 90

```solidity
pragma solidity 0.6.12;
pragma experimental ABIEncoderV2;

import "forge-std/Test.sol";
import {mockERC20} from "../src/mocks/mockERC20.sol";
import {mockWETH} from "../src/mocks/mockWETH.sol";
import {StakingRewards} from "../src/farm/StakingRewards.sol";

contract DeflationaryERC20 is mockERC20 {
    function _transfer(address sender, address recipient, uint256 amount) internal override {
        amount = amount * 9 / 10;
        super._transfer(sender, recipient, amount);
    }
}

contract StakingRewardsTest is Test {
    StakingRewards public stakingRewards;
    DeflationaryERC20 public mockToken;
    mockWETH public WETH;

    function setUp() public {
        mockToken = new DeflationaryERC20();
        WETH = new mockWETH();
        stakingRewards = new StakingRewards(address(mockToken), address(WETH));
    }

    function testStakeDeflationary() public {
        mockToken.mint(address(this), 100);
        mockToken.approve(address(stakingRewards), 100);
        stakingRewards.stake(100);
        uint256 balance = stakingRewards.balanceOf(address(this));
        uint256 actualBalance = mockToken.balanceOf(address(stakingRewards));

        assertFalse(actualBalance == balance);
    }
}

```

It is expected that calls of the form `approve(spender, amount)` fail if the address in `spender` is the zero address.

It is expected that calls of the form 
- `transfer(recipient, amount)`
- `transferFrom(from, dest, amount)`

fail if the recipient address is the zero address.

When there are no stakers, the total staked amount is 0. Then when updating rewards, only the reward update time is updated.

```solidity=165
    modifier updateReward(address account) {
        rewardPerTokenStored = rewardPerToken();
        lastUpdateTime = lastTimeRewardApplicable();
```
```solidity=62
    function rewardPerToken() public view override returns (uint256) {
        if (_totalSupply == 0) {
            return rewardPerTokenStored;
        }
```

If the duration of time for when there are no stakers is denoted as `T`, then `rewardRate * T` rewards will be forever locked in the contract. This is because when updating the reward amount, the contract does not handle rewards that have not been earned due to no stakers.

```solidity=135
    function notifyRewardAmount(uint256 reward, uint256 newRewardsDuration) external override onlyRewardsDistribution updateReward(address(0)) {
        rewardsDuration = newRewardsDuration;
        if (block.timestamp >= periodFinish) {
            rewardRate = reward.div(rewardsDuration);
        } else {
            uint256 remaining = periodFinish.sub(block.timestamp);
            uint256 leftover = remaining.mul(rewardRate);
            rewardRate = reward.add(leftover).div(rewardsDuration);
        }
```

For example, suppose the first reward duration was for 10 days and 100 tokens are given as rewards each day. If there are no stakers for the first 5 days and the first staker stakes on the 6th day, then the 500 tokens meant to be given as rewards will be locked inside the contract forever.

When adding rewards to the staking contract, a check is performed to ensure the contract's balance is sufficient for the stated reward.

```solidity=149
        uint balance = rewardsToken.balanceOf(address(this));
        require(rewardRate <= balance.div(rewardsDuration), "Provided reward too high");
```

However, in the situation where the reward token is the same as the staking token, this check can be bypassed as `balance` will include tokens that users have staked.

### Proof of Concept

The below test done in forge does the following:
1. A staking contract is created where the staking token and reward token are the same
2. 100 tokens are staked to the contract
3. `notifyRewardAmount()` is called and succeeds, stating 100 reward tokens are available even though no reward tokens were transferred to the contract

```solidity
pragma solidity 0.6.12;
pragma experimental ABIEncoderV2;

import "forge-std/Test.sol";
import {mockERC20} from "../src/mocks/mockERC20.sol";
import {StakingRewards} from "../src/farm/StakingRewards.sol";

contract StakingRewardsTest is Test {
    StakingRewards public stakingRewards;
    mockERC20 public mockToken;

    function setUp() public {
        mockToken = new mockERC20();
        stakingRewards = new StakingRewards(address(mockToken), address(mockToken));
    }

    function testBypassRewardCheck() public {
        mockToken.mint(address(this), 100);
        mockToken.approve(address(stakingRewards), 100);
        stakingRewards.stake(100);
        stakingRewards.notifyRewardAmount(100, 1);
    }
}
```

The signature malleability is possible within the Elliptic Curve cryptographic system. An Elliptic Curve is symmetric on the X-axis, meaning two points can exist with the same `X` value. In the `r`, `s` and `v` representation this permits us to carefully adjust `s` to produce a second valid signature for the same `r`, thus breaking the assumption that a signature cannot be replayed in what is known as a replay-attack.

### Proof of Concept

Using the signature obtained from https://solidity-by-example.org/signature/, the below unit test in `forge` shows that two different `s` and `v` values can produce the same address for one `r` value.

```solidity
pragma solidity 0.6.12;
pragma experimental ABIEncoderV2;

import "forge-std/Test.sol";

contract ECRecoverTest is Test {
    function setUp() public {
    }

    function getMessageHash(
        address _to,
        uint _amount,
        string memory _message,
        uint _nonce
    ) public pure returns (bytes32) {
        return keccak256(abi.encodePacked(_to, _amount, _message, _nonce));
    }

    function getEthSignedMessageHash(bytes32 _messageHash)
        public
        pure
        returns (bytes32)
    {
        return
            keccak256(
                abi.encodePacked("\x19Ethereum Signed Message:\n32", _messageHash)
            );
    }

    function testVerifySignature() public {
        bytes32 r = 0x993DAB3DD91F5C6DC28E17439BE475478F5635C92A56E17E82349D3FB2F16619;
        bytes32 s_a = 0x6F466C0B4E0C146F285204F0DCB13E5AE67BC33F4B888EC32DFE0A063E8F3F78;
        bytes32 s_b = 0x90B993F4B1F3EB90D7ADFB0F234EC1A3D43319A763C0117891D4548691A701C9;
        uint8 v_a = 0x1B; // 27
        uint8 v_b = 0x1C; // 28
        
        bytes32 messageHash = getMessageHash(0x14723A09ACff6D2A60DcdF7aA4AFf308FDDC160C, 123, "coffee and donuts", 1);
        bytes32 ethSignedMessageHash = getEthSignedMessageHash(messageHash);

        address owner_a = ecrecover(ethSignedMessageHash, v_a, r, s_a);
        address owner_b = ecrecover(ethSignedMessageHash, v_b, r, s_b);
        assertEq(owner_a, owner_b);
    }
```

The following interface is unused:
```solidity=4
import '../interfaces/IOKCSwapPair.sol';
```

The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.

Quick Summary

OKX DEX suffered an access control exploit on Dec 12, 2023, resulting in a loss of 2,390,976 USD worth of assets, including USDT,
USDC, and WETH.




Details of the Exploit

OKX DEX, a trading aggregator for cross-chain transactions, experienced an access control exploit on December 12, 2023. The proxy
admin owner upgraded the DEX proxy contract to a new implementation contract, which may have led to the compromise of the private
key of the OKX DEX. After the upgrade, tokens started being stolen from the platform. The stolen native ETH was distributed
between three addresses, while the rest of the stolen stable coins were bridged to Arbitrum and Avalanche chains via Stargate
Bridge. The DEX proxy was removed from OKX's platform's trusted list following the incident. The total loss amounted to 2,390,976
USD worth of assets, including 142,034 USDT, 475,929 USDC, and 799.77 WETH.




Block Data Reference

Attackers Addresses:

https://etherscan.io/address/0xFacf375Af906f55453537ca31fFA99053A010239

https://etherscan.io/address/0x0519efacb73a1f10b8198871e58d68864e78b8a5




Funds Holders as of Dec 14, 2023:

https://etherscan.io/address/0xfe55502a57f388a69602b2780071b759a520468f

https://etherscan.io/address/0x22a2931cb2a7b782d65b2b5562829e84d941b0f0

https://etherscan.io/address/0xa15fe801dd5fd31a684c444b6980dbaf0c78d5ad




Malicious Transactions:

https://etherscan.io/tx/0x7a9c03576158b08bd896293fffcb11dd2fcc09c3d896335affee9968b4a1db5c

https://etherscan.io/tx/0x78bfe55b18e53513b5c17869f39cc9cc21f3d6d2b6b44d1ceb9762789449dcd2

https://etherscan.io/tx/0xf69cf6cc56849be0ee93e8651fdf3622639b7a99e1a620c744f3fef8a5743236




Stargate Bridging Transactions:

https://etherscan.io/tx/0xd2b424b17e0959d260df748ef9d8b62120abe64d011ae68e00e8d3874d99ed28

https://etherscan.io/tx/0x444fe10b2487c2c3cfa79fd878f3c0c5f520a9b4e94a44a6ce8e5a2bd8d9dd8b