code-423n4 / 2022-06-connext-findings

1 stars 0 forks source link

enrollCustom() is too permissive #257

Closed code423n4 closed 1 year ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/4dd6149748b635f95460d4c3924c7e3fb6716967/contracts/contracts/core/connext/helpers/TokenRegistry.sol#L141

Vulnerability details

Issue: enrollCustom() is too permissive.

Consequences: in case the owner gets compromised on one chain, this can result in funds being exploitable on other chains by enrolling malicious tokens.

Affected Code

File: TokenRegistry.sol
141:   function enrollCustom( // @audit-info [HIGH] 
142:     uint32 _domain,
143:     bytes32 _id,
144:     address _custom
145:   ) external override onlyOwner { 
146:     // update mappings with custom token
147:     _setRepresentationToCanonical(_domain, _id, _custom);
148:     _setCanonicalToRepresentation(_domain, _id, _custom);
149:   }

Mitigations

Consider enrolling a custom token in a time lock manner where you enroll a new token and wait X amount of days until it becomes available. In this way there can be alerts in place that listen on chain for unauthorized enrolls in case the owner gets compromised.

LayneHaber commented 2 years ago

TokenRegistry is not in scope

0xleastwood commented 1 year ago

Agreed, this is not-in-scope. Plus I think its fair to assume an honest governance.