CertiK reviewed AMPL (Ampleforth) on 6 May 2020: 10 findings, 0 critical; none marked fixed.
Report hosted on skynet.certik.com.
Trustblock does not review code.
Statuses are the security firm’s own, from its report.
| Severity | Open | Acknowledged | Fixed |
|---|---|---|---|
| Critical | 0 | 0 | 0 |
| High | 0 | 0 | 0 |
| Medium | 0 | 0 | 0 |
| Low | 10 | 0 | 0 |
The “externalCall” function returns a variable of type “bool” as stated in its function signature. Internally, an in-memory variable is declared called “result” of type “bool” which is assigned the result of the low-level “call” invocation and then explicitly returned as the last statement of the function body
This Exhibit relies on whether Exhibit 2 is applied on the codebase. If Exhibit 2 is applied, this Exhibit can be safely ignored. In the aforementioned line, whenever a new “Transaction” struct is constructed the hard-coded value of “true” is assigned to its “enabled” member. This will presently cause a 32-byte write operation to be conducted as the memory needs to be initialized to a non-zero value.
The loop included in the aforementioned lines iterates through an array of dynamic size that could potentially lead to the function being impossible to execute if many transactions are included or if the gas cost of the transactions collectively exceeds the block’s gas limit.
The “TransactionFailed” event contains a “uint” variable that represents the index of the transaction that failed. However, this variable may be misleading under certain circumstances as transaction ordering in Ethereum is up to the miners of the blocks. As an example, should a single block contain the following transaction ordering, the “index” variable will be misleading and lead to an out-of-bound access on both the previous and current block of the blockchain: 1. Inclusion of new transaction in transaction queue. This will increase the transaction queue length and add the final transaction to the end. 2. Execution of “rebase” and failed execution of the newly included transaction, leading to the “TransactionFailed” event being emitted with the “index” variable being the last element of the array. 3. The deletion of the last transaction in the transaction queue. This will decrease the transaction queue length and remove the final transaction that was previously executed but failed. The above sequence will lead to an “index” being emitted that is otherwise inaccessible. The above scenario, although unlikely, can occur should the owner of the contract accidentally include a transaction and attempt to remove it with users invoking “rebase” in the process.
The “Transaction” struct included on the Orchestrator contract can be optimized to occupy one less slot, significantly reducing the gas cost of all types of transactions relating to it such as read and write operations.
Whenever the “length” of an array is reduced, Solidity internally calls the “delete” operator on the last element of the array and as such, it is unnecessary to explicitly call it.
The “externalCall” function contains a “transferValueWei” variable in its signature referring to the amount of wei that will be transferred in the external call. This function is labelled as “internal” and is only invoked in a single location, line 55, where the “transferValueWei” argument is hard-coded as zero.
The aforementioned line contains an assignment of the zero value casted to an “address” type to the “orchestrator” variable of the contract. This type of assignment is unnecessary as Solidity, akin to many other programming languages, assigns a default value to uninitialized variables which in the case of addresses is the zero address.
The “externalCall” function contains a “dataLength” variable in its signature referring to the length of the “bytes” dynamic array that will be transferred in the external call. This is unnecessary because, as mentioned in comment line 138, the length of the “bytes” is already included in the variable at the first location of the “bytes” pointer.
The function “transactionsLength” is meant to return, as its name implies, the “length” of the “transactions” storage array. It is generally considered ill-practise to declare a dot-accessor in camel-case format as the function name.
The report names a repository or files. Open the report to see its scope.
1,044 projects reviewed · 1,375 reports · latest May 2025
See CertiK’s auditsSomething wrong on this page? A missing report, a wrong date, a finding that was fixed since. Tell us and we check it against its source.
Tell us