CertiK reviewed STFX on 12 Dec 2022: 16 findings, 0 critical; 3 marked fixed, 13 acknowledged.
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 | 0 | 1 | 0 |
| Medium | 0 | 1 | 0 |
| Low | 0 | 11 | 3 |
In the contract **gmx/Reader.sol**, the role **owner** has authority over the following functions: - function setDex(), to set `dex` address. - function setOwner(), to change the owner of the contract. Any compromise to the **owner** account may allow a hacker to take advantage of this authority and change the config for the protocol. In the contract **StfxVault.sol**, the role **admin** has authority over the following functions: - function closePosition(), to close the stfx position. - function cancelOrder(), to cancel the pending order. - function distributeProfits(), to distribute profits to users. - function closeLiquidatedVault(), to close the stfx liquidated by the dex. - function cancelVault(), to cancel the empty or expired stfx. - function cancelStfAfterOpening(), to cancel the pending stfx. - function cancelStfAfterPositionDeadline(), to cancel the stf after a month and close the position as a market order. - function pause() and unpause(), to set the pause status. **AND** The role `owner` has authority over the following functions: - function setCapacityPerStf(), to set the max capacity of collateral which can be raised per stf. - function setMinInvestmentAmount(), to set the min investment of collateral an investor can invest per stf. - function setMaxInvestmentAmount() and setMaxLeverage(), to set max leverage and invest amount. - function setMinLeverage(), to set min leverage. - function setMaxFundraisingPeriod(), to set the max fundraising period. - function setMaxDeadlineForPosition(), to set the max deadline a position can be open for an stf. - function setManagerFee(), to set the manager fee percent. - function setProtocolFee(), to set the protocol fee percent. - function setOwner(), to set owner. - function setStfxImplementation(), to set the new stfx implementation. - function setReader(), to set the reader. - function setFundDeadline(), to set the fundDeadline. - function setUsdc(), to set usdc address. - function setWeth(), to set weth address. - function setAdmin(), to set admin. - function setReferrerCode(), to set referralCode. - function setIsManagingFund(), to set the isManagingFund. - function withdraw(), withdrawToken(), withdrawFromStf(), to withdraw fund from the contracts. Any compromise to the **owner** and **admin** accounts may allow a hacker to take advantage of this authority and bring unpredictable damages to the project. In the contract `Presale` 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 bring unpredictable damages to the project.  In the contract `STFXToken` 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 bring unpredictable damages to the project. 
The bool variable `closed` is defaulted to false. And when `block.chainid == 10`, it is not set to true. So the status of the `stf` is not changed after the function `closePosition()` is called.
We understand that if anyone can become a manager, the project team should have control over managers. When the admin receives complaints and decides to take over the stf, the admin himself/herself may not represent the wishes of every investor that trusts the manager. Investors join the manager's stf, which means they trust the manager. But the admin has not been able to obtain the approval of investors, as long as the admin interferes with the stfx, there is bound to be a part of the users opposed.
According to the logic, the protocol on different chains will interact with different `stfx` implementations. While it is not recommended to use the check in hard code. It's recommended to directly convert the address to `IStfxGmx` or `IStfxPerp` in constructor of the contract.
The contract `vault` receives collateral tokens from the user and allows the manager to invest. The collateral token should not be deflationary tokens because the amount recorded is not the really received amount but the parameter amount(before tax). When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged (and burned) transaction fee. As a result, this may not meet the assumption behind these low-level asset-transferring routines and will bring unexpected balance inconsistencies.
The manager can choose leverage to open the position. In the high-leverage position, a margin call is required inevitably. But the protocol seems not to support the operation, since the manager can not flexibly use the funds and he/she can only open one position.
According to the comment of the function `cancelOrder()`, the parameter `_isOpen` is used to decide which type of order to close. When `_isOpen` is false, it's assumed that the function should revert if the status is OPENED.
According to the logic of the protocol, the function `getPrice()` should return the instant price of the `baseToken`. While the price is not from Price Oracle but the function of the `baseToken` itself. Since the implementation of `baseToken` is not found in scope. We hope the team to elaborate more on the price consult mechanism of `baseToken`.
Functions that update state variables should emit relevant events as notifications.
The modifier `onlyManager` is not used in the contract.
The comment at line 122 is redundant because it describes the condition that could happen in closing positions but it appears in the function `openPosition()`. And it is not aligned with the surrounding codes.
The contract is serving as the underlying entity to interact with 3rd party **ClearingHouse** protocols. The scope of the audit treats 3rd party entities as black boxes and assumes their functional correctness. However, in the real world, 3rd parties can be compromised and this may lead to lost or stolen assets. Additionally, upgrades of 3rd parties can possibly create severe impacts, such as increasing fees of 3rd parties, migrating to new LP pools, etc. In file `gmx/Reader.sol`: - **IGmxVault(dex.vault)**.`shortableTokens()` (line 49) - **IGmxVaultPriceFeed(vaultPriceFeed)**.`getPrice()` (line 59) In file `gmx/Stfx.sol`: - **IGmxPositionRouter()**.`minExecutionFee()` (line 123, 194, 249) - **IGmxRouter()**.`approvePlugin()` (line 138, 210) - **IGmxOrderBook()**.`createIncreaseOrder()` (line 139) - **IGmxPositionRouter()**.`createIncreasePosition()` (line 154) - **IGmxOrderBook()**.`createDecreaseOrder()` (line 211) - **IGmxPositionRouter()**.`createDecreasePosition()` (line 217) - **IGmxOrderBook()**.`cancelIncreaseOrder()` (line 246) - **IGmxOrderBook()**.`cancelDecreaseOrder()` (line 259) - **IGmxReader()**.`getAmountOut()` (line 338, 350) - **IGmxRouter()**.`swap()` (line 357) In file `perp/Reader.sol`: - **IMarketRegistry()**.`hasPool()` (line 23) - **IBaseToken()**.`getIndexPrice()` (line 35) In file `perp/Stfx.sol`: - **IClearingHouse()**.`openPosition()` (line 77, 91) - **IclearingHouse()**.`closePosition()` (line 111) In file `StfxVault.sol`: - **IGmxVault()**.`getPosition()` (line 230)
The linked `transfer()`/`transferFrom()` invocations do not check the return value of the function call, which should yield `true` in the case of proper ERC-20 implementation.
The report names a repository or files. Open the report to see its scope.
Repository and files the report names (7)
1,044 projects reviewed · 1,393 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