fix: detect assembly delegatecall in controlled-delegatecall

[BhariGowda]

Summary

controlled-delegatecall reports a delegatecall or callcode whose destination is tainted by user input. It only ever looked at source-level calls, so the same contract written with inline assembly produced no finding at all.

Root cause

controlled_delegatecall() walked function.low_level_calls only:

for ir in function.low_level_calls:
    if ir.function_name in [
        "delegatecall",
        "callcode",
    ]:
        if is_tainted(ir.destination, function.contract):
            ret.append(ir.node)

Inline assembly delegatecall and callcode are parsed into Yul nodes and become SolidityCall operations, not LowLevelCall (slither/solc_parsing/yul/parse_yul.py:46). They are therefore absent from function.low_level_calls entirely, so the loop never saw them regardless of how the destination was derived. There was no taint decision being made and rejected, the call simply was not in the collection being iterated.

Measured on the same contract written both ways with solc 0.8.15: the source-level form is reported on master, the assembly delegatecall and the assembly callcode forms are both silent.

slither/detectors/attributes/locked_ether.py:71-86 already matches assembly calls by checking SolidityCall against the Yul signatures alongside the LowLevelCall names. This PR does the same here.

Changes

slither/detectors/statements/controlled_delegatecall.py: add a second loop over function.solidity_calls matching the two Yul signatures, taint-checking the callee address. That address is argument index 1 in both, since Yul delegatecall(g, a, in, insize, out, outsize) and callcode(g, a, v, in, insize, out, outsize) agree on the position. The existing low-level loop and the reporting are unchanged, and the existing name list is pulled out as a constant for symmetry with the new one.

Scope decision on proxy fallbacks, stated up front

Assembly delegatecall is how essentially every proxy is written, so I measured the false-positive surface before proposing this. Five shapes on solc 0.8.15:

shape	reported
assembly delegatecall, destination from a function parameter	yes
assembly callcode, destination from a function parameter	yes
assembly delegatecall to a hardcoded address	no
EIP-1967 proxy, sload of a constant slot	no
naive proxy, address public implementation plus sload(implementation.slot) in the fallback	yes

The last row is the one that matters. OpenZeppelin-style proxies and anything else reading a fixed EIP-1967 slot are unaffected, because the slot is a constant and the loaded value is not tainted. A proxy holding its implementation in an ordinary state variable is reported.

That is not new noise introduced by this PR. I checked the equivalent contract on unmodified master, with an admin-gated upgradeTo and a fallback doing implementation.delegatecall(msg.data) at source level, and master already reports it today. This change makes the assembly form behave the same as the source form rather than creating a new class of finding, which is also consistent with the detector's MEDIUM confidence.

I deliberately did not extend the existing if contract.is_upgradeable_proxy and f.is_protected(): continue guard with an f.is_fallback case. A proxy's fallback is unprotected by construction, so that suppression would fire on every proxy, including one whose upgradeTo has no access control at all, which is a real and serious bug this detector should keep reporting. Narrowing the fallback case is a separate judgement call about the detector's existing source-level behaviour and belongs in its own change, not smuggled in behind a parsing fix.

Fixtures

tests/e2e/detectors/test_data/controlled-delegatecall/{0.6.11,0.7.6,0.8.15}/assembly_delegatecall.sol, following the existing fixture's good/bad shape: a state variable tainted from msg.sender used as the destination of both an assembly delegatecall and an assembly callcode, plus a constant-initialised state variable used the same way. The snapshots contain the two tainted cases and not the constant one, so the fixture pins that the taint check is still doing work and the arm is not matching unconditionally.

Same pre-0.6 limitation as the sibling change in #3087: with solc below 0.6.0 the whole assembly block arrives as a single node holding the source as one opaque string in node.inline_asm, so there are no Yul nodes and no SolidityCall to match. That parsing gap is tracked in #1775. The fixtures therefore start at 0.6.11.

This detector had no 0.8.x fixture at all, stopping at 0.7.6, so a 0.8.15/ directory is added. Happy to drop that and keep the fixture to the two existing versions if you would rather not widen the matrix in a bug fix.

Test plan

Run with uv per CONTRIBUTING.md.

• Baseline pytest tests/e2e/detectors/test_detectors.py -k ControlledDelegateCall on master: 4 passed.
• After the change: 7 passed. Three new cases, one per version.
• Reverted the detector file and kept the new fixtures: 3 failed, 4 passed. The three failures are exactly the three new cases.
• Full pytest tests/e2e/detectors/test_detectors.py: 384 passed on master, 387 passed with the change. No existing snapshot modified.
• ruff check slither tests: clean. ruff format --check on both touched Python files: already formatted.
• Full pytest tests on both master and this branch produces an identical set of 6 failures and 10 errors, all vyper cases with no vyper compiler installed plus the two read-storage tests that need a node.

Sibling PR: #3087 fixes the same SolidityCall versus LowLevelCall gap in unprotected-upgrade. The two are independent and can be reviewed in either order.

[CLAassistant]

All committers have signed the CLA.

[glarregay-tob]

Reviewed this and it looks correct to me.

Verified the three new cases pass, the full detector suite is 387 passed with no existing snapshot modified, and it merges cleanly with #3087 in either order (391 pass with both applied). I also reproduced your false-positive table — EIP-1967 and hardcoded-address proxies stay clean, the naive address public implementation proxy is reported, and master already reports the source-level equivalent of that one. Taint reaches the new check whether the destination is a parameter, a Yul temp, or a nested sload(...) inline.

Tagging @smonicas for the remaining issues: whether to keep the new 0.8.15/ fixture directory, and leaving the is_fallback suppression out.