CertiK reviewed OKX (OKX - Audit 3) on 16 May 2023: 27 findings, 0 critical; 5 marked fixed, 22 open.
Report hosted on skynet.certik.com.
Trustblock does not review code.
Statuses are the security firm’s own, from its report.
| Severity | Open | Acknowledged | Fixed |
|---|---|---|---|
| Critical | 0 | 0 | 0 |
| High | 1 | 0 | 0 |
| Medium | 0 | 0 | 0 |
| Low | 21 | 0 | 5 |
In the function `_exeForks()`, multiple swaps are performed, with each swap swapping `batchAmount * weight / 10000` tokens. ```solidity=86 function _exeForks(address payer, uint256 batchAmount, RouterPath calldata path) private { address fromToken = bytes32ToAddress(path.fromToken); // execute multiple Adapters for a transaction pair uint256 pathLength = path.mixAdapters.length; for (uint256 i = 0; i < pathLength; ) { bytes32 rawData = bytes32(path.rawData[i]); address poolAddress; bool reserves; uint256 weight; assembly { poolAddress := and(rawData, _ADDRESS_MASK) reserves := and(rawData, _REVERSE_MASK) weight := shr(160, and(rawData, _WEIGHT_MASK)) } require(weight >= 0 && weight <= 10000, "weight out of range"); uint256 _fromTokenAmount = (batchAmount * weight) / 10000; _transferInternal(payer, path.assetTo[i], fromToken, _fromTokenAmount); ``` Since each swap swaps a portion of `batchAmount`, it is expected for all of `batchAmount` to be used. This would require the sum of all weights to be 10000, but no such check exists, allowing the possibility that some tokens are unused or tokens meant for a different batch are used.
The function `_smartSwapInternal()` simultaneously iterates over the arrays `batchesAmount` and `batches` to be used when calling `_exeHop()`. ```solidity=316 for (uint256 i = 0; i < batches.length; ) { // execute hop, if the whole swap replacing by pmm fails, the funds will return to dexRouter _exeHop(payer, batchesAmount[i], batches[i], extraData); unchecked { ++i; } } ``` This suggests that both arrays `batchesAmount` and `batches` have to be the same length, but there is no check that this is the case.
The function `PMMV2SwapByInvest()` invokes the `_PMMV2Swap()` to perform a token swap via the private market maker. ```solidity=538 function PMMV2SwapByInvest( address receiver, PMMLib.PMMBaseRequest memory baseRequest, PMMLib.PMMSwapRequest calldata request ) external payable nonReentrant returns (uint256 returnAmount) { require(request.fromToken != _ETH, "Invalid source token"); if (baseRequest.fromTokenAmount == 0) { baseRequest.fromTokenAmount = IERC20(request.fromToken).balanceOf(address(this)); } return _PMMV2Swap(address(this), receiver, baseRequest, request); } ``` The argument `fromTokenPayer` is set as the contract address in line 547 of function `PMMV2SwapByInvest()`. ```solidity function _PMMV2Swap( address fromTokenPayer, address receiver, PMMLib.PMMBaseRequest memory baseRequest, PMMLib.PMMSwapRequest calldata request ) internal returns (uint256 returnAmount) { ... ``` However, the `baseRequest.fromTokenAmount` is updated to the `IERC20(request.fromToken).balanceOf(address(this))` only if the input `baseRequest.fromTokenAmount == 0`. In the case that the `baseRequest.fromTokenAmount` is nonzero, it will not be updated. Considering the case the `baseRequest.fromTokenAmount` is larger than `IERC20(request.fromToken).balanceOf(address(this))`, the swap will fail due to insufficient funds in the contract when line 175 of function `_pmmSwapInternal()` is executed. `_pmmSwapInternal()` in **Web3-DEX-dev/contracts/8/PMMRouter.sol** ```solidity=171 if (fromNative) { IWETH(_WETH).deposit{ value: actualAmountRequest }(); IERC20(_WETH).safeTransfer(request.payer, actualAmountRequest); } else if(fromTokenPayer == address(this)) { IERC20(request.fromToken).safeTransfer(request.payer, actualAmountRequest); } else { IApproveProxy(_APPROVE_PROXY).claimTokens(request.fromToken, fromTokenPayer, request.payer, actualAmountRequest); } ```
In the function `_tryPmmSwap()`, the variable `subIndex` is assigned the length of the `bytes` array `pmmRequest.extension`. ```solidity=202 assembly{ subIndex := mload(add(extension, 0x0)) } ``` The operation `add(extension, 0x0)` is redundant meaning `mload(extension)` will produce the same result.
In both functions `_performERC1155Transfer()` and `_performERC721Transfer()`, the variable `success` is assigned with 0 if the passed `token` is not a contract. After that, the `success` is assigned with the return value of the `call()`, which means the `success` is overwritten by the return value of `call()`. In the case that the passed `token` is not a contract, it should return false immediately and should not continue the execution of the remaining code.
The report names a repository or files. Open the report to see its scope.
Repository and files the report names (4)
1,044 projects reviewed · 1,375 reports · latest May 2025
See CertiK’s auditsSomething wrong on this page? A missing report, a wrong date, a finding that was fixed since. Tell us and we check it against its source.
Tell us