Introduction
CoinFabrik was asked to audit the contracts for the Limit Order Protocol and Fixed Rate Swap projects. First we will provide a summary of our discoveries and then we will show the details of our findings.
Summary
The contracts audited are from the LimitOrderProtocol repository at https://github.com/1inch/limit-order-protocol and the FixedRateSwap repository at https://github.com/1inch/fixed-rate-swap. The audit is based on the commits 9d118307df7acc3bcef73407f3964acd6aa0f35c and d05dc6d0a4e1d5fe1b8cb2a1a4d098b7e64cc30c respectively.
Contracts
The audited contracts include the following, plus their helper and libraries:
- [fixed-rate-swap]/contacts/FixedRateSwap.sol: main contact implementing the fixed swap functions.
- [limit-order-protocol]/contracts/LimitOrderProtocol.sol: main contract inheriting Limit Order and Limit Order RFQ functions.
- [limit-order-protocol]/contracts/OrderMixin.sol: contract defining Limit Order functions.
- [limit-order-protocol]/contracts/OrderRFQMixin.sol: contract defining Limit Order RFQ functions.
Analyses
We analyzed whether the code is secure, if best practices are met, and the logic for compliance with the protocol’s model.
The following analyses for typical vulnerabilities were performed:
- Misuse of the different call methods
- Integer overflow errors
- Division by zero errors
- Outdated version of Solidity compiler
- Front running attacks
- Reentrancy attacks
- Misuse of block timestamps
- Softlock denial of service attacks
- Functions with excessive gas cost
- Missing or misused function qualifiers
- Needlessly complex code and contract interactions
- Poor or nonexistent error handling
- Failure to use a withdrawal pattern
- Insufficient validation of the input parameters
- Incorrect handling of cryptographic signatures
Severity Classification
Security risks are classified as follows:
- Critical: These are issues that we manage to exploit. They compromise the system seriously.
They must be fixed immediately. - Medium: These are potentially exploitable issues. Even though we did not manage to exploit them or their impact is not clear, they might represent a security risk in the near future. We suggest fixing them as soon as possible.
- Minor: These issues represent problems that are relatively small or difficult to take advantage of but can be exploited in combination with other issues.
These kinds of issues do not block deployments in production environments. They should be taken into account and be fixed when possible. - Enhancement: These kinds of findings do not represent a security risk. They are best practices that we suggest to implement.
This classification is summarized in the following table:
SEVERITY | EXPLOITABLE | ROADBLOCK | TO BE FIXED |
Critical | Yes | Yes | Immediately |
Medium | In the near future | Yes | As soon as possible |
Minor | Unlikely | No | Eventually |
Enhancement | No | No | Eventually |
Issues Found by Severity
Critical severity
No issues were found.
Medium severity
No issues were found.
Minor severity
No issues were found.
Enhancements
Incorrect Comment
The comment for func_301JL5R() is
// keccak256(“func_733NCGU(address,address,uint256,address,uint256,bytes)”) == 0x23b872dd (IERC20.transferFrom)
Which is incorrect. The hash value matches the new function. Correct to
// keccak256(“func_301JL5R(address,address,uint256,address,uint256,bytes)”) == 0x23b872dd (IERC20.transferFrom)
Conclusion
We found the contracts to be simple and straightforward and have an adequate amount of documentation. These contracts improve upon contracts which have been previously audited, only adding functionality in the case of Limit Order Protocol. No security issues were found.