OKX - Audit 3

CertiKFiles named in the report

CertiK reviewed OKX (OKX - Audit 3) on 16 May 2023: 27 findings, 0 critical; 5 marked fixed, 22 open.

No open critical findings
Open the reportOpen OKX’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings27

Statuses are the security firm’s own, from its report.

SeverityOpenAcknowledgedFixed
Critical000
High100
Medium000
Low2105
LowLack of Check on WeightsFixed

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.

LowLack of Length Check on BatchesFixed

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.

LowPossible Inconsistency Caused by Check of `baseReFixed

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); } ```

LowPossibly Incorrect Assignment to Variable `subIndFixed

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.

LowVariable `success` Is OverwrittenFixed

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.

Contracts in scope

No deployed address is recorded for this audit.

The report names a repository or files. Open the report to see its scope.

Repository and files the report names (4)

  • contracts/adapters/uniswap/UniswapV2AdapterMain.sol
  • contracts/Aggregator/OkxNFTMarketAggregator.sol
  • contracts/Entrance.sol
  • contracts/8/DexRouter.sol

Part of OKX’s security record

OKX
Audits5 audits by 1 firmLast audit May 2024, by CertiK.
Critical findingsNo open critical findingsAcross the 5 reports, 1 critical finding was reported. It is marked fixed by the firm that reported it.
IncidentIncident 12 Dec 2023Source: De.Fi incident database.
Open OKX’s security record

Security firm

CertiK

1,044 projects reviewed · 1,375 reports · latest May 2025

See CertiK’s audits

Something 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