[HIGH] Using `msg.sender` instead of `_msgSender()` allows relayers to receive funds intended for the signed caller

[cygent-dev]

Security Finding

Field	Value
Severity	HIGH
Category	access_control
Repository	1inch/solidity-utils
Confidence	52%

Description

This finding is an access control vulnerability caused by using msg.sender instead of _msgSender() in code that executes as part of BySig-signed delegatecalls. BySig implements signature-based delegation: it validates a signature, pushes the authorized signer into an internal transient stack and then executes the signed payload using address(this).functionDelegateCall(sig.data). During that delegatecall, EVM-level msg.sender remains the transaction originator (the relayer), while BySig::_msgSender() returns the logical signer from the transient stack. This difference is central to the vulnerability.

[Truncated - see PR for full details]

Location

• BySig.sol:L120-L130 in bySig (BySig)
• Rescuable.sol:L27-L29 in rescueFunds (Rescuable)
• UniERC20.sol:L91-L95 in uniTransferFrom (UniERC20)

Impact

An attacker acting as relayer can divert funds (ETH or ERC20) from contracts to themselves when a signer delegates a call. A single mistaken call (e.g., rescues of the contract balance) can result in full loss of funds controlled by the contract. Affected parties include contract owners, privileged signers, and any users relying on signed actions.

Recommendation

Use _msgSender() everywhere for logical caller checks and for determining recipients in delegatecall-executed signed calls. Replace msg.sender with _msgSender() in Rescuable::rescueFunds and audit other call sites. For library functions (like UniERC20::uniTransferFrom) that require the logical sender, accept an explicit sender parameter (pass _msgSender() from the calling contract) instead of reading msg.sender inside the library.

Suggested minimal fix for Rescuable (change msg.sender to _msgSender()):

[Truncated - see PR for full details]

---

Created by CARA Security Audit via Cygent