Project
One World
NFTDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

_msgSender() fails to return the transaction's sender when msg.sender == address(this)

elser17

Summary

_msgSender() fails to return the transaction's sender when msg.sender == address(this)

Vulnerability Details

MembershipFactory._msgSender() should return the EOA address of the function caller when msg.sender == address(this), but this is not correct because everything it does is return the last 20 bytes of msg.data, which is expected to be the mentioned address but will never be.

This happens because if a low level call is done, the context changes to the own call, and so does the msg.data. Therefore, when joinDAO() or upgradeTier() are called with msg.data == address(this), _msgSender returns tierIndex and fromTierIndex params respectively.

See PoC below.

Impact

_msgSender() does not work as expected and does not return the expected address when msg.data == address(this), making joinDAO() or upgradeTier() revert in this cases.

Tools Used

Remix, Manual review

PoC

Recommendations

The utility of the _msgSender() function is not very clear, but if it expects to return the EOA which called the function, simply return tx.origin.

function _msgSender()
internal
view
override
returns (address sender)
{
if (msg.sender == address(this)) {
- bytes memory array = msg.data;
- uint256 index = msg.data.length;
- assembly {
// Load the 32 bytes word from memory with the address on the lower 20 bytes, and mask those.
- sender := and(
- mload(add(array, index)),
- 0xffffffffffffffffffffffffffffffffffffffff
- )
+ sender = tx.origin;
}
} else {
sender = msg.sender;
}
return sender;
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
0xbrivan2 Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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