feat: Add Function module to detect functions returning msg.sender directly or via alias

[sidarth16]

Summary

This PR introduces a new method is_returning_msg_sender() in the Function class, enabling detection of Solidity functions that return msg.sender, either directly or through variable aliasing.

---

Motivation

In many smart contract patterns, msg.sender is returned through internal functions, often after assignment to local variables or via wrapper functions like _msgSender(). Detecting such patterns is crucial for accurate static analysis—particularly when assessing function protection or authorization flows.

This utility provides foundational support for enhancing other modules involving msg.sender (in future PRs ).

---

What’s Covered

The newly added method Function.is_returning_msg_sender() returns True if:

• The function directly returns msg.sender
• The function returns a variable that was directly or transitively assigned from msg.sender

Examples:

• Direct return :

function _msgSender() internal view returns (address) {
    return msg.sender;
}

• Transitive Aliasing:

function _myMsgSender() internal view returns (address) {
    address a = msg.sender;
    address b = a;
    return b;
}

What's not Covered

• Returns via internal function calls, even if those functions return msg.sender:

function _getUser() internal view returns (address) {
        return _msgSender();  // _msgSender() returns msg.sender
}

Support for such recursive or wrapper logic will be addressed in a future update.

Closes #2755

[sidarth16]

The following Solidity contract was used for local testing of the is_returning_msg_sender() utility:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract TestMsgSender {
    address public owner;

    constructor() {
        owner = msg.sender;
    }

    // ✅ Should be detected
    function directSender() internal view returns (address) {
        return msg.sender;
    }

    // ✅ Should be detected
    function aliasSender() internal view returns (address) {
        address a = msg.sender;
        return a;
    }

    // ✅ Should be detected
    function multiAliasSender() internal view returns (address) {
        address a = msg.sender;
        address b = a;
        address c = b;
        return c;
    }

    // ✅ Should be detected
    function reassignSender() internal view returns (address) {
        address a = msg.sender;
        address b = a;
        a = b;
        return a;
    }

    // ✅ Should be detected
    function _getSender() internal view returns (address) {
        return msg.sender;
    }

    // ❌ Should NOT be detected (msg.sender conversions)
    function conversionSender() internal view returns (address) {
        address a = msg.sender;
        address b = address(uint160(uint256(uint160(a))));
        return b;
    }

    // ❌ Should NOT be detected (nested internal call)
    function returnsViaInternal() internal view returns (address) {
        return _getSender();
    }

    // ❌ Should NOT be detected (returns unrelated var)
    function unrelatedReturn() internal view returns (address) {
        address a = owner;
        return a;
    }

    // ❌ Should NOT be detected (does not return msg.sender)
    function notMsgSender() internal view returns (address) {
        return address(this);
    }

    // ❌ Should NOT be detected (does not return address)
    function notAddress() internal view returns (uint256) {
        address a = msg.sender;
        return 7;
    }
}

Test Code

from slither.slither import Slither

sl = Slither('test_msgsender.sol')
contract = sl.contracts[0]
for func in  contract.functions:
    print(f'{func.canonical_name}    :  {func.is_returning_msg_sender()}' )

Output

TestMsgSender.constructor()       	:  False
TestMsgSender.directSender()       	:  True
TestMsgSender.aliasSender()       	:  True
TestMsgSender.multiAliasSender()       	:  True
TestMsgSender.reassignSender()       	:  True
TestMsgSender._getSender()       	:  True
TestMsgSender.conversionSender()       	:  False
TestMsgSender.returnsViaInternal()     	:  False
TestMsgSender.unrelatedReturn()       	:  False
TestMsgSender.notMsgSender()       	:  False
TestMsgSender.notAddress()       	:  False

[dguido]

Code Review

Good feature addition for issue #2755. The implementation follows existing patterns in the codebase (similar to is_protected()).

What the PR does well

1. Follows existing patterns - Uses caching (_is_returning_msg_sender) like other methods in the Function class
2. Well-documented - Clear docstring explaining what's covered and what's not (e.g., doesn't handle returns via internal function calls)
3. Handles transitive aliasing - The assignment_map[lval] = assignment_map.get(rval, rval) pattern correctly tracks chains like a = msg.sender; b = a; return b
4. Type-safe - Only tracks address-typed assignments

Issues to address

1. Missing automated tests

The PR includes manual test results in a comment but no automated unit tests. Please add tests to tests/unit/ covering:

• Direct return of msg.sender
• Single alias (address a = msg.sender; return a)
• Transitive alias chain
• Reassignment case
• Negative cases (returns unrelated variable, returns via internal call)

Example test structure:

def test_is_returning_msg_sender(solc_binary_path):
    # Test cases from your manual testing
    ...

2. PR contains unrelated changes

The diff includes many unrelated files (Dockerfiles, CI workflows, pyproject.toml) that appear to be merge artifacts from the 'dev' branch. Consider rebasing to only include the function.py changes relevant to this feature.

Minor observations

Control flow limitation: The algorithm doesn't distinguish between different control flow paths. For example:

function foo(bool x) returns (address) {
    address a;
    if (x) {
        a = msg.sender;
    } else {
        a = address(0);
    }
    return a;
}

This would return True because a was assigned from msg.sender at some point, even though it might not be the actual return value. This is acceptable given the documented limitations, but worth noting.

Verdict

The implementation is sound, but please add automated tests before merging. The existing manual test cases in the PR comment would make excellent unit tests.

🤖 Generated with Claude Code

[sidarth16]

Hi @smonicas @dguido
Just following up when you have a moment. All checks are passing now.
Please let me know if there’s anything you’d like me to adjust.
Thanks!