Date of engagement: 3rd April 2024 - 8th April 2024
-
Timur GüvenkayaPrincipal advisor
Engagement Dashboard
Audit results
Guvenkaya conducted a security and code improvement assessment of the NEKO airdrop script from 3rd April 2024 to 8th April 2024. During this engagement, a total of 11 findings were reported. 3 of the findings were medium, and the remaining were informational severity. All major issues are fixed by the Trove Labs team.
Engagement record
- Client
- Trove Labs - Neko
- System
- NEKO airdrop script
- Review period
- 3rd April 2024 - 8th April 2024
- Paths in scope
- 1
- Principal advisor
- Timur Güvenkaya
Timeline
- Review start3rd April 2024
- Draft report10th April 2024
- Final report8th June 2024
Findings by severity
Finding status at final report
Remediation complexity
Impact
Likelihood
Advisory and Client
The advisory
Guvenkaya specializes in security for digital assets and financial infrastructure, from smart contracts and protocols to applications, infrastructure, custody and cryptography.
Our team, including the sitting Heads of Security at Gauntlet and Agora, brings experience from Kraken, OpenZeppelin, Binance, ZKsync, Nillion, Invicti and EY. Across the team: 200+ security reviews, $30B+ in volume secured, 13 published cryptography research papers, and credentials including OSCP, OSWE and CISM.
- Every reviewer has a public bio and is named before you sign. We match expertise to your system, with a principal leading scoping and checking findings.
The client
NEKO is NEAR Protocol's first meme coin, a crypto content creation hub and AI app developer. NEKO was born in early 2022 as the first meme coin on NEAR Protocol, aiming to become a key community support node. The Neko community was cultivated during the most challenging market conditions that NEAR has ever seen. When the chips were down, the Neko community grew stronger; showing up and creating educational crypto content every day. By the end of 2023, Neko was making waves. Growing into one of the largest NEAR native communities with over 23,500 unique holders!
Scope and Review Boundaries
Review depths
- Deep
- Line-by-line review with explicit attack hypotheses, adversarial reasoning about state transitions, and reproduction attempts for each hypothesis that survived reading.
- Supporting
- The code was read for its effect on a reviewed flow. Behaviour outside that effect was not assessed.
- Targeted
- Entry points were exercised or triaged against the flow's security properties rather than read in full.
This is the code we read, and how closely. 1 paths are in scope. The original report counts 204 lines of code reviewed. Each path links to the file at the reviewed commit.
| Path | Review depth | Scope note |
|---|---|---|
airdrop-tool-mac/ | Deep | Airdrop Script |
Out of scope
The audit will include, but is not limited to, reviewing the code for security vulnerabilities, coding practices, and architecture. The audit does not include a review of the dependencies.
Assessment Framework
Guvenkaya rates findings by likelihood and impact so remediation can be prioritized by technical risk, business context, and the decision the engagement is meant to support.
Rating chain applied to every finding
- 01 Likelihood Likely, possible or rare
- 02 Impact Severe, moderate or negligible
- 03 Severity Derived from the matrix below
- 04 Remediation complexity Simple, moderate or difficult
- 05 Status Not fixed, fixed, acknowledged or scheduled
Impact
| Impact | What it means here |
|---|---|
| Severe | Exploitation could result in critical loss or compromise, such as full system control, substantial financial loss, or severe reputational damage. |
| Moderate | Exploitation may lead to limited data loss, partial compromise, moderate financial impact, or noticeable degradation of services. |
| Negligible | Exploitation has minimal impact, such as minor data exposure without significant consequences or slight inconvenience without substantial disruption. |
Likelihood
| Likelihood | What it means here |
|---|---|
| Likely | The vulnerability is easily discoverable and not overly complex to exploit. |
| Possible | The vulnerability presents some challenges either in discovery or in the complexity of the attack. |
| Rare | The vulnerability is either very difficult to discover or complex to exploit, or both. |
This matrix provides a nuanced view, taking into account both the ease of discovering a vulnerability and the complexity involved in exploiting it.
| Attack Complexity \ Discovery Ease | Obvious | Concealed | Hidden |
|---|---|---|---|
| Complex | Possible | Rare | Rare |
| Moderate | Likely | Possible | Rare |
| Straightforward | Likely | Possible | Possible |
Severity
| Severity | What it means here |
|---|---|
| Critical | Assigned to vulnerabilities with severe impact and a likely likelihood of exploitation. |
| High | For vulnerabilities with either severe impact but only a possible likelihood, or moderate impact with a likely likelihood. |
| Medium | Used for vulnerabilities with severe impact but a rare likelihood, moderate impact with a possible likelihood, or negligible impact with a likely likelihood. |
| Low | For vulnerabilities with moderate impact and rare likelihood, or negligible impact with a possible likelihood. |
| Informational | The lowest severity rating, typically for vulnerabilities with negligible impact and a rare likelihood of exploitation. |
| Likelihood \ Impact | Severe | Moderate | Negligible |
|---|---|---|---|
| Likely | Critical | High | Medium |
| Possible | High | Medium | Low |
| Rare | Medium | Low | Informational |
Remediation complexity
This measures how difficult it is to remediate a finding once it has been identified.
| Remediation complexity | What it means here |
|---|---|
| Simple | Patches or fixes are readily available and easily implemented. |
| Moderate | Requires some time and resources to remediate, but well within the capabilities of most organizations. |
| Difficult | Remediation requires significant resources, specialized skills, or substantial changes to systems or architecture. |
Finding status
This tracks the client's response to each finding and whether remediation has been completed, accepted, or scheduled.
| Status | What it means here |
|---|---|
| Not Fixed | Indicates that the vulnerability has been identified but no remedial action has been taken yet. This status is crucial for newly discovered vulnerabilities or those awaiting prioritization. |
| Fixed | This status is applied when the vulnerability has been successfully remediated. It implies that appropriate measures, such as patching, configuration changes, or architectural modifications, have been implemented to resolve the issue. |
| Acknowledged | This status is used for vulnerabilities that have been recognized, but for various reasons (such as risk acceptance, cost, or other business decisions), have not been fixed. It indicates that the risk posed by the vulnerability is known and has been consciously accepted. |
| Scheduled | This status indicates that the vulnerability has been acknowledged and a plan is in place to fix it in the future. It signifies that while remediation hasn't yet occurred, the issue has been prioritized and is part of the planned development roadmap. |
Findings Summary
Findings register
Detailed Findings
GUV-1: Lack of Error Handling in Airdrop Script
Description
We noticed that the airdrop script does not handle errors originating from the smart contract. For instance, if we try to send more than the available balance, the script won't fail. We will get a false sense of success. In addition, the script used both try/catch and .then/.catch syntaxes, leading to inconsistent code and difficulty in reading the code. There was also the introduction of more detailed and specific errors for all async operations. For example, if there is an error on reading a JSON file, we get the following error: Error reading JSON file: $error , whereas before, there was general errors such as Retrying trasnfer to : $id
Recommendation
We propose logging smart contract fails for each account and moving those account to error folder.
Remediation
The Trove Labs team has fixed the issue in the following releases: v1.1.0 & v1.2.0
GUV-2: Several Issues In The Promise Handling
Description
We noticed that in promises, there was the use of the resolve method for all cases, even when there is an error, the syntactically correct method to use is rejected when there is an error within a promise. There was also a misleading log that says Retrying transfer to id : $id , where then the Promise is resolved and the code moves on to the next Promise, in this sense, there is no retry logic implemented. The use of Promise.all while improving performance; it can introduce a wide variety of issues due to its parallel execution nature.
NONCE MANAGEMENT
Since the script is manually managing the nonce, using Promise.all can cause failures because of its non sequential nature, Which can lead to nonce conflicting and eventually cause failure.
ERROR HANDLING
Due to its parallel execution nature, error handling can be difficult to manage.
await fs.writeFile( join(__dirname, "..", "data", listName + ".json"), JSON.stringify(list), ); resolve(); } catch (e) { console.log("Retrying Transfer to ", id); resolve(); } }); index++; promiseArray.push(promise);}await Promise.all(promiseArray); Recommendation
We propose refactoring the code to ensure that the Promise errors are properly handled and that the nonce is managed correctly.
Remediation
The Trove Labs team has fixed the issue in the following release: v1.0.0
GUV-3: Finished Folder Is Always Empty
Description
We noticed that after the completion of the airdrop, the json file which is supposed to contain the list of accounts that have been airdropped is empty. It happens because after the verification of the account existence, the account is deleted from the original file. Since the original file is used to write the list of accounts that have been airdropped to the final folder, the file is empty.
const accountData = await nearConnection.connection.provider .query(`account/${id}`, "") .catch(async (_accountError) => { console.log("Invalid account Id:", id); await fs.appendFile( join(__dirname, "..", "error", listName + ".txt"), `${id}:${transferAmount.toString()} `, ); delete list[id]; await fs.writeFile( join(__dirname, "..", "data", listName + ".json"), JSON.stringify(list), ); }); Recommendation
We propose either refactoring the application to avoid deleting the date from the original list or directly on each iteration appending to the file in the finished folder after clearing the old data.
Remediation
The Trove Labs team has fixed the issue in the following release: v1.0.0
GUV-4: Repetition and Lack of Modularity in the Script
Description
We noticed that the original file had a lot of repetitive blocks in terms of prompting the user and file handling. This makes it harder to read, debug, and implement changes to the script, increasing the risk of introducing bugs by mistake.
Recommendation
Consider refactoring the code to make it more modular and reduce repetition. Example of refactored script is shown below:
import { connect, Contract, keyStores, utils, transactions } from "near-apijs";import inquirer from "inquirer";import fs from "fs/promises";import os from "os";import path from "path";import { fileURLToPath } from "url";import BN from "bn.js";// Custom types =====================type TokenMethod = { ft_balance_of: (args: { account_id: string }) => Promise<string>; storage_balance_of: (args: { account_id: string }) => Promise<any>; ft_transfer: (args: { args: { receiver_id: string; amount: string }; amount: string; }) => Promise<void>; storage_deposit: (args: { args: { account_id: string; registration_only: boolean }; amount: string; }) => Promise<string>;}; type TokenContract = Contract & TokenMethod;type List = { [walletId: string]: number;};// ============================// Global variables ==========================const __filename = fileURLToPath(import.meta.url);const __dirname = path.dirname(__filename);const homedir = os.homedir();const CREDENTIALS_DIR = ".near-credentials";const credentialsPath = path.join(homedir, CREDENTIALS_DIR);const myKeyStore = new keyStores.UnencryptedFileSystemKeyStore(credentialsPath);const finishedList: List = {};// ==========================// Configs ===============const CONFIG = { networkId: "mainnet", nodeUrl: "https://rpc.mainnet.near.org", walletUrl: "https://wallet.mainnet.near.org", helperUrl: "https://helper.mainnet.near.org", explorerUrl: "https://explorer.mainnet.near.org", keyStore: myKeyStore,};const contractId = "ndc.tkn.near";const PROMPT_MESSAGES = { accountId: "Enter Account Id used for shooting Token:", listName: "Enter the json file name:", amount: "Enter the amount of TOKEN per NFT to shoot:", confirm: (listName: string) => `Start Shooting Token to the list [ ${listName} ]?`,}; // ==========// Helper methods =================async function promptUser( message: string, type: string = "input", name: string = "input",): Promise<string> { const { [name]: value } = await inquirer.prompt([{ type, name, message }]); return value;}async function getContract(account: any): Promise<TokenContract> { return new Contract(account, contractId, { viewMethods: ["ft_balance_of", "storage_balance_of"], changeMethods: ["ft_transfer", "storage_deposit"], }) as TokenContract;}async function readJsonFile( filePath: string,): Promise<{ [walletId: string]: number }> { try { const data = await fs.readFile(filePath, "utf-8"); return JSON.parse(data); } catch (error) { console.error("Error reading JSON file:", error); throw error; }} async function writeJsonFile(filePath: string, data: object): Promise<void> { try { await fs.writeFile(filePath, JSON.stringify(data, null, 2)); } catch (error) { console.error("Error writing JSON file:", error); throw error; }}async function appendToFile(filePath: string, content: string): Promise<void> { try { await fs.appendFile(filePath, content); } catch (error) { console.error("Error appending to file:", error); throw error; }}async function moveFile(source: string, destination: string): Promise<void> { try { await fs.rename(source, destination); } catch (error) { console.error("Error moving file:", error); throw error; }}// ================= async function processTransfers( account: any, contract: TokenContract, listName: string, amount: BN, ): Promise<void> { const listFilePath = path.join(__dirname, "..", "data", `${listName}.json`); const list: List = await readJsonFile(listFilePath); const signerPubKey = await account.connection.signer.getPublicKey( account.accountId, CONFIG.networkId, ); const accessKey: any = await account.connection.provider.query( `access_key/${account.accountId}/${signerPubKey}`, "", ); let nonce = accessKey.nonce + 1; for (const [walletId, balance] of Object.entries(list)) { try { const transferAmount = new BN(balance).mul(amount); // Check if the account exists try { await account.connection.provider.query(`account/${walletId}`, ""); } catch (error) { console.log("Invalid account Id:", walletId); await appendToFile( path.join(__dirname, "..", "error", `${listName}.txt`), `${walletId}:${transferAmount.toString()}`, );ß delete list[walletId]; await writeJsonFile(listFilePath, list); continue;} console.log("Valid account Id:", walletId); console.log("Checking Storage Balance..."); const storageBalance = await contract.storage_balance_of({ account_id: walletId, }); const actions = []; if (!storageBalance) { console.log("Registering Storage", walletId); actions.push( transactions.functionCall( "storage_deposit", { account_id: walletId, registration_only: true }, new BN("30000000000000"), new BN(utils.format.parseNearAmount("0.00125")!), ), ); } actions.push( transactions.functionCall( "ft_transfer", { receiver_id: walletId, amount: transferAmount.toString() }, new BN("30000000000000"), new BN("1"), ), ); const recentBlockHash = utils.serialize.base_decode(accessKey.block_hash); const transaction = transactions.createTransaction( account.accountId, signerPubKey, contractId, nonce++, actions, recentBlockHash, ); console.log("Transferring to", walletId); const signedTransaction = await transactions.signTransaction( transaction, account.connection.signer, account.accountId, CONFIG.networkId, ); await account.connection.provider.sendTransaction(signedTransaction[1]); finishedList[walletId] = list[walletId]; delete list[walletId]; await writeJsonFile(listFilePath, list); } catch (error) { console.error( `Error processing transfer to wallet : ${walletId} with error:`, error, ); } } if (Object.keys(list).length === 0) { const finishedFilePath = path.join( __dirname, "..", "finished", `finished_${listName}.json`, ); await writeJsonFile(finishedFilePath, finishedList); console.log("✔️✔️✔️✔️✔️✔️✔️✔️✔️ DONE ✔️✔️✔️✔️✔️✔️✔️✔️✔️"); } else { console.log( "========================== Partially Done ==============================", ); await processTransfers(account, contract, listName, amount);}} async function main() { const nearConnection = await connect(CONFIG); const accountId = await promptUser(PROMPT_MESSAGES.accountId); const account = await nearConnection.account(accountId); const contract = await getContract(account); const listName = await promptUser(PROMPT_MESSAGES.listName); const amount = new BN(await promptUser(PROMPT_MESSAGES.amount)); const confirmTransfer = await promptUser( PROMPT_MESSAGES.confirm(listName), "confirm", ); if (!confirmTransfer) { console.log("Okay, bye!"); return; } await processTransfers(account, contract, listName, amount); } main().catch((error) => { console.error("Error in main:", error); process.exit(1); }); Remediation
The Trove Labs team has fixed the issue in the following release: v1.0.0
GUV-5: Insufficient Retry Functionality In The Script
Description
We noticed that the script does not have a proper retry mechanism with backoff strategy in case of a failure. Transactions can fail due to RPC issues such as timeout error and invalid nonce errors. It is crucial to ensure that those transactions are repeated MAX_REPEAT_TIMES.
Recommendation
Make sure to implement a retry mechanism with a backoff strategy to handle transaction failures. It is crucial to only retry RPC related errors since errors originating from the smart contract cannot be resolved by retrying.
Remediation
The Trove Labs team has fixed the issue in the following releases: v1.0.0 & v1.1.0
GUV-6: Redundant Script Folder Should Be Removed
Description
We noticed that both airdrop-tool-mac and airdrop-tool-windows contain exactly the same script. This redundancy can lead to confusion and potential issues in the future. It is recommended to remove one of the folders and keep only one version of the script.
Recommendation
Consider leaving a single folder and renaming it to airdrop-tool.
Remediation
The Trove Labs team has fixed the issue in the following release: v1.0.0
GUV-7: Redundant RPC Call To Check Whether The Account Exists
Description
We noticed that the script calls an RPC to check whether the account is valid and exists. However, it is possible to avoid this call by checking on Transaction error returned. If error relates to the accunt not being found, then the account does not exist. The only thing we can do beforehand is to check whether the account id follows the correct format, which is fast.
const accountData = await nearConnection.connection.provider .query(`account/${id}`, "") .catch(async (_accountError) => { console.log("Invalid account Id:", id); await fs.appendFile( join(__dirname, "..", "error", listName + ".txt"), `${id}:${transferAmount.toString()} `, ); delete list[id]; await fs.writeFile( join(__dirname, "..", "data", listName + ".json"), JSON.stringify(list),);}); ... Recommendation
Consider matching on error after the transaction is sent and beforehand check whether the account id format correct.
Remediation
The Trove Labs team has fixed the issue in the following release: v1.1.0
GUV-8: Lack Of Feedback On Completed Transactions
Description
We noticed that after the transaction is sent we do not get any feedback whether it was successful or failed. Also, we do not get a transaction hash which can be used to check the status later or share with the user
Recommendation
Consider logging whether the transaction was successful or failed with a link to a block explorer where the user can check the status of the transaction if needed.
Remediation
The Trove Labs team has fixed the issue in the following release: v1.0.0
GUV-9: Missing Zero Value Check On Airdrop Amount
Description
We noticed that script operator is allowed to put zero as the number of airdrops. Having zero as the number of airdrops leads to the redundant execution of the script. It is recommended to add a check to prevent the operator from executing the script with zero airdrops.
Recommendation
Consider adding a check to prevent the operator from executing the script with zero as the number of airdrops.
Remediation
The Trove Labs has acknowledged the issue.
GUV-10: Error File Should Be Replaced Instead Of Appended Into
Description
We noticed that ther error txt file is appended on each run. This can lead to duplicate entries if the script is executed multiple times. It is recommended to replace the file on each run.
Recommendation
Consider replacing the error file on each run instead of appending to it.
Remediation
The Trove Labs team has fixed the issue in the following release: v1.0.0
GUV-11: Insufficient Documentation
Description
We noticed that the documentation does not mention that the script requires both error and final folders to be present in the root directory. It is recommended to add this information to the documentation to avoid any confusion.
Recommendation
Consider adding information to the documentation that the script requires both error and final folders to be present in the root directory.
Remediation
The Trove Labs team has fixed the issue in the following release: v1.0.0
The web version and the PDF (23 pages) are generated from the same source. The report as first delivered is kept in the public reports archive.