Redacted Cartel contest - kyteg's results

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/PirexGmx.sol#L505-L516

Vulnerability details

Impact

If an ERC20 token that take fees on transfers is GMX-witelisted and used to mint and stake GLP, there will be a mismatch between the token amount received by the PirexGmx.sol contract and the value of GLP minted and staked by PirexGmx.sol. This will slowly drain the contract of funds.

Proof of Concept

If the depositGlp() function is called by a user to deposit GLP using a GMX-whitelisted token, the PirexGmx.sol contract request funds transfer from the token contract via the SafeTransferFrom() function on line 506 (https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/PirexGmx.sol#L506). If the token contract takes fees on transfer, the actual balance of tokens transfered to the PirexGmx.sol contract is less than tokenAmount.

The contract then mints and stakes tokens based on the tokenAmount variable on lines 510-115, which is smaller than the amount of tokens received by the contract. https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/PirexGmx.sol#L510-L515.

Tools Used

Manual code inspection

Recommended Mitigation Steps

There is no easy mitigation for this problem, as there are no easy ways to check the received token balance after a transaction. A possible solution is to check the contracts token balance before and after the transaction to determine the actual amount of tokens received, but this may lead to unforeseen race condition vulnerabilities. Another possible solution is to keep a separate token whitelist separate from the GMX contract, and only allow non fee-on-transfer tokens on the whitelist.