code-423n4 / 2023-10-opendollar-findings

10 stars 7 forks source link

Upgraded Q -> 2 from #385 [1699689442074] #451

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

Judge has assessed an item in Issue #385 as 2 risk. The relevant finding follows:

[L-04] ODSafeManager.allowSAFE function enables any allowed address to add/remove other allowed addresses Details The ODSafeManager.allowSAFE function is meant by design to allow/disallow any address to manage the safe; this can be first accessed by the SAFE owner to add an allowed address; then this allowed address will have the ability to add/remove (allow/disallow) any address on the SAFE.

As the allowed addresses on the safe can call vital functions on the SAFE such as transferring its collateral and internal coins;any malicious address can overtake the SAFE and drain it.

So it's recommended to enable the safe owner only from accessing the ODSafeManager.allowSAFE function on his own SAFE.

Proof of Concept ODSafeManager.allowSAFE function

function allowSAFE(uint256 _safe, address _usr, uint256 _ok) external safeAllowed(_safe) { address _owner = _safeData[_safe].owner; safeCan[_owner][_safe][_usr] = _ok; emit AllowSAFE(msg.sender, _safe, _usr, _ok); } Recommended Mitigation Steps Update ODSafeManager.allowSAFE function to enable the SAFE owner only from allowing/disallowing any address on the SAFE (similar to the design of allowHandler function).

c4-judge commented 1 year ago

MiloTruck marked the issue as duplicate of #171

c4-judge commented 1 year ago

MiloTruck marked the issue as satisfactory