Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Discrepancy between intended logic and actual code implementation

0xseverityhunter

Summary

Failure to use the && operator judiciously could halt important functions from being carrried out.

Vulnerability Details

This modifier checks if msg.sender is preMarketsAddr or deliveryPlaceAddr which are contract addresses of PreMarkets.sol and DeliveryPlace.sol respectively.

modifier onlyRelatedContracts(
ITadleFactory _tadleFactory,
address _msgSender
) {
/// @dev check caller is pre markets or delivery place
address preMarketsAddr = _tadleFactory.relatedContracts(
RelatedContractLibraries.PRE_MARKETS
);
address deliveryPlaceAddr = _tadleFactory.relatedContracts(
RelatedContractLibraries.DELIVERY_PLACE
);
​
if (_msgSender != preMarketsAddr && _msgSender != deliveryPlaceAddr) {
revert CallerIsNotRelatedContracts(_msgSender);
}
​
_;
}

The if statement on line 13 above, implements an AND operator which would only pass if the two checks pass, which isn't possible as the preMarketsAddr and deliveryPlaceAddr contract addresses are seperate.

Impact

The onlyRelatedContractsmodifier is implemented twice in the TokenManager.sol contract these functions would always revert

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L64

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L118

Tools Used

Manual Analysis

Recommendations

The ANDoperator should be replaced with the ORoperator instead

-- if (_msgSender != preMarketsAddr && _msgSender != deliveryPlaceAddr) {
++ if (_msgSender != preMarketsAddr || _msgSender != deliveryPlaceAddr) {
​
Updates

Lead Judging Commences

0xnevi Lead Judge
11 months ago
0xnevi 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.