Neko / Trove LabsAirdrop Script Security Review

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

PROPOSED SOLUTION

We propose logging smart contract fails for each account and moving those account to error folder.

REMEDIATION - FIXED

The Trove Labs team has fixed the issue in the following releases: v1.1.0 & v1.2.0

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.

script:runMain:airdrop-tool-mac/src/index.ts

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);

PROPOSED SOLUTION

We propose refactoring the code to ensure that the Promise errors are properly handled and that the nonce is managed correctly.

REMEDIATION - FIXED

The Trove Labs team has fixed the issue in the following release: v1.0.0

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.

script:runMain:airdrop-tool-mac/src/index.ts

            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), ); });

PROPOSED SOLUTION

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 - FIXED

The Trove Labs team has fixed the issue in the following release: v1.0.0

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.

PROPOSED SOLUTION

Consider refactoring the code to make it more modular and reduce repetition. Example of refactored script is shown below:

script:runMain:airdrop-tool-mac/src/index.ts

            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>;
            };

script:runMain:airdrop-tool-mac/src/index.ts

          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} ]?`,
          };

script:runMain:airdrop-tool-mac/src/index.ts

            // ==========
            // 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;
              }
            }

script:runMain:airdrop-tool-mac/src/index.ts

            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;
              }
            }
            // =================

script:runMain:airdrop-tool-mac/src/index.ts

            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;}

script:runMain:airdrop-tool-mac/src/index.ts

            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,
            );

script:runMain:airdrop-tool-mac/src/index.ts

              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);}}

script:runMain:airdrop-tool-mac/src/index.ts

      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 - FIXED

The Trove Labs team has fixed the issue in the following release: v1.0.0

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.

PROPOSED SOLUTION

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 - FIXED

The Trove Labs team has fixed the issue in the following releases: v1.0.0 & v1.1.0

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.

PROPOSED SOLUTION

Consider leaving a single folder and renaming it to airdrop-tool.

REMEDIATION - FIXED

The Trove Labs team has fixed the issue in the following release: v1.0.0

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.

script:runMain:airdrop-tool-mac/src/index.ts

            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),);}); ...

PROPOSED SOLUTION

Consider matching on error after the transaction is sent and beforehand check whether the account id format correct.

REMEDIATION - FIXED

The Trove Labs team has fixed the issue in the following release: v1.1.0

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

PROPOSED SOLUTION

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 - FIXED

The Trove Labs team has fixed the issue in the following release: v1.0.0

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.

PROPOSED SOLUTION

Consider adding a check to prevent the operator from executing the script with zero as the number of airdrops.

REMEDIATION - ACKNOWLEDGED

The Trove Labs has acknowledged the issue.

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.

PROPOSED SOLUTION

Consider replacing the error file on each run instead of appending to it.

REMEDIATION - FIXED

The Trove Labs team has fixed the issue in the following release: v1.0.0

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.

PROPOSED SOLUTION

Consider adding information to the documentation that the script requires both error and final folders to be present in the root directory.

REMEDIATION - FIXED

The Trove Labs team has fixed the issue in the following release: v1.0.0