I Took the Smart Contract Auditing Challenge with DetectBox — Join Me

Introduction

DetectBox represents an emerging and prospective auditing marketplace that introduces a fresh approach. This platform provides projects and protocols the unique opportunity to independently choose auditors who will take up the crucial responsibility of evaluating their projects. In stark contrast to the conventional auditing firms, which often obscure the identities of the individuals involved, DetectBox promotes transparency. DetectBox permits anyone with the requisite qualifications to apply for the role of an auditor. Selection involves the completion of a rigorous screening process. This inclusive approach ensures that skilled individuals from various backgrounds have the chance to contribute their expertise and become an integral part of the auditing process.

Project Overview

The project, Komet’s DropZone, facilitates the streamlined batch transfer of diverse token categories. These encompass ETH, ERC20, ERC721, and ERC1155 tokens. Traditional batch transaction execution, when conducted individually, is marked by a bad user experience. In return for accessing this functionality, users are mandated to pay the service fees.

The Collaboration

Throughout the audit, I had the distinct privilege of collaborating closely with devScrooge. Our collaborative endeavours proved instrumental in pin pointing and mitigating an array of issues, encompassing 1 Critical, 2 High, 2 Medium, 6 Low and other findings. The proactive engagement of Komet’s team in rectifying these matters was truly commendable. Our collaborative efforts included insightful meetings, with one dedicated to understanding the intricacies of the codebase, and another focused on addressing audit-related inquiries. Collectively, this experience unfolded seamlessly, fostering a productive and enriching partnership.

Key Discoveries

Let’s delve into a couple of noteworthy discoveries that could prove beneficial for fellow developers and aspiring auditors.

a. Usage of an incorrect version of Ownable library

The contract is designed as an upgradeable proxy contract. However, the current iteration employs a non-upgradeable variant of the Ownable library. In a conventional setup with a non-upgradeable Ownable library, the deployer automatically becomes the default owner during the constructor’s execution. However, the proxy-based upgradeability system there is no usage of constructors within upgradeable contracts.

As a direct consequence of this constraint, no owner will emerge upon the proxy contract’s deployment. Consequently, all functions to be accessible only by the owner (onlyOwnerfunctions) will remain beyond reach.

import "@openzeppelin/contracts/access/Ownable.sol";

function setMaxLimit(uint _maxLength) public onlyOwner {
    MAX_LENGTH = _maxLength;
}

function setPlatformFees(uint _maxLimit, uint _platformFees) public onlyOwner {
    fee_limit = _maxLimit;
    platform_fee = _platformFees;
}

b. Missing Input Validation

The getPlatformFee function currently allows users to specify the number of decimals as an input parameter. However, this design opens the door for potential abuse, where a malicious user could input a large value to set decimal_amount to zero, effectively circumventing the fee requirement. A straightforward resolution for this vulnerability would involve retrieving the decimal value from the IERC20 interface, rather than permitting users to provide it manually.

function getPlatformFee(
 uint256 _amount,
 bool _isNative,
 uint256 decimals
 ) internal {
   uint256 decimal_amount = _amount / 10 ** decimals;
   if (decimal_amount >= fee_limit) {
   //rest of the code
   }
}

c. Unbounded Arrays and Out-of-Gas Reverts

When dealing with unbounded arrays as input, there is an inherent risk of encountering out-of-gas reverts. To mitigate this risk, We proposed that developers conduct thorough unit testing and establish an upper limit for array length. For instance, setting a maximum length of 25 would result in a transaction revert with a “max length” error if a user attempts to provide an array length exceeding this threshold.

Importance of Unit Testing

I strongly encourage all developers to prioritize the creation of basic unit tests for their code. This practice not only enhances auditors’ comprehension of the code but also streamlines the process of developing proof of concepts (PoCs). By eliminating the need to write test cases from scratch, developers can effectively expedite the auditing process.

Conclusion & Acknowledgments

I would like to extend my heartfelt gratitude to devScrooge & detectbox.io for their invaluable collaboration throughout this audit. I eagerly anticipate more opportunities for such fruitful partnerships in the future.

To read the full audit report. Follow the link — https://github.com/UNSNARL/audit-reports/blob/main/Dropzone_Komet_Security_Assessment.pdf