DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: medium
Invalid

Incorrect Sender Assignment using `LibTractor._user()` in `SiloFacet`

Summary

The deposit function allows users to deposit ERC20 tokens into the Silo contract. It calculates the Bean Denominated Value (BDV) for the deposited tokens, updates the deposit records for the user, and mints Stalk tokens to the user. The function handles different modes of funds transfer (INTERNAL, EXTERNAL, EXTERNAL_INTERNAL, INTERNAL_TOLERANT) and ensures reentrancy protection.

Vulnerability Details

The deposit function has a potential issue related to the flow of the LibTractor._user() result being used as the sender to receiveToken and _deposit functions. This flow raises concerns about the potential for unintended consequences, specifically:

  • According to the LibTractor._user() implementation, it returns activePublisher if set, otherwise defaults to msg.sender. This means that the actual sender of the transaction can be overridden based on the state of activePublisher.

  • The receiveToken function is called with the result of LibTractor._user() as the sender. This function is responsible for transferring tokens into the contract. If activePublisher is set, tokens will be deducted from activePublisher's balance instead of the transaction initiator (msg.sender), which can lead to confusion and unexpected behavior.

  • Similar concerns extend to the _deposit function, where the same sender (result of LibTractor._user()) is used. This consistency might be intentional but should be carefully reviewed to ensure it aligns with the intended logic and security requirements.

See the following code:

function deposit(
address token,
uint256 _amount,
LibTransfer.From mode
)
external
payable
fundsSafu
noSupplyChange
noOutFlow
nonReentrant
mowSender(token)
returns (uint256 amount, uint256 _bdv, int96 stem)
{
amount = LibTransfer.receiveToken(IERC20(token), _amount, LibTractor._user(), mode);
(_bdv, stem) = _deposit(LibTractor._user(), token, amount);
}

Impact

This issue can lead to fund misallocation, unintended token transfers, and potential security vulnerabilities if not managed correctly.

Tools Used

Manual Review

Recommendations

Review the logic in deposit, receiveToken, and _deposit functions to ensure that the flow of tokens and the determination of the transaction sender (msg.sender vs activePublisher) are intentional and align with the contract's security and operational requirements.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.