EthBridgeOptimized Manual Review Findings

EBO-01M: Improper Validation of EIP-20 Transfer

Description:

The ETH bridge implementation validates EIP-20 transfers by ensuring that the output of transfer and transferFrom operations is not false. However, this will cause many tokens to fail to properly function as tokens such as USDT (Tether) do not properly conform to the standard and instead return no value.

Example:

Copy
114if (
115    IERC20(ETHTOKEN).transferFrom(msg.sender, address(this), amount) ==
116    false
117) revert TransferFromFailed();

Recommendation:

We advise a safe wrapper library to be utilized instead that opportunistically evaluates the return value of transfers only if a bool exists, such as OpenZeppelin's SafeERC20 implementation.

Alleviation:

The DAFI Protocol team evaluated this exhibit but opted not to apply a remediation for it in the current iteration of the codebase.

EBO-02M: Insecure Elliptic Curve Recovery Mechanism

Description:

The ecrecover function is a low-level cryptographic function that should be utilized after appropriate sanitizations have been enforced on its arguments, namely on the s and v values. This is due to the inherent trait of the curve to be symmetrical on the x-axis and thus permitting signatures to be replayed with the same x value (r) but a different y value (s).

Example:

Copy
201function recoverSigner(bytes32 message, bytes memory sig)
202    internal
203    pure
204    returns (address)
205{
206    (uint8 v, bytes32 r, bytes32 s) = splitSignature(sig);
207
208    return ecrecover(message, v, r, s);
209}

Recommendation:

We advise them to be sanitized by ensuring that v is equal to either 27 or 28 (v ∈ {27, 28}) and to ensure that s is existent in the lower half order of the elliptic curve (0 < s < secp256k1n ÷ 2 + 1) by ensuring it is less than 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A1. A reference implementation of those checks can be observed in the ECDSA library of OpenZeppelin and the rationale behind those restrictions exists within Appendix F of the Yellow Paper.

Alleviation:

The signature verification checks are now properly enforced in the codebase along with corresponding error declarations.

EBO-03M: Potential Invalidation of Arbitrary Transactions

Description:

The adjustment of the threshold can be performed arbitrarily by the owner and will cause all pending transactions that exactly match it to fail if the threshold is increased.

Example:

Copy
86function setThreshold(uint256 _newThreshold) external onlyOwner {
87    if (!(_newThreshold >= 2 ))
88        revert InvalidThreshold();
89    threshold = _newThreshold;
90}

Recommendation:

We advise this trait of the system to be re-evaluated and the threshold adjustment to be guarded by additional security checks, such as ensuring that it is at most equal to a percentage of the total signers of the system which should be tracked in a dedicated variable.

Alleviation:

The DAFI Protocol team evaluated this exhibit but opted not to apply a remediation for it in the current iteration of the codebase.