Prepared by:
HALBORN
Last Updated 05/15/2024
Date of Engagement by: April 5th, 2024 - April 17th, 2024
100% of all REPORTED Findings have been addressed
All findings
3
Critical
0
High
0
Medium
1
Low
1
Informational
1
Pink Elements engaged Halborn to conduct a security assessment on their backend applications beginning on 2024-04-05 and ending on 2024-04-17. The security assessment was scoped to the assets provided to the Halborn team.
The team at Halborn was provided one week and a half for the engagement and assigned a full-time security engineer to verify the security of all the applications. The security engineer is a penetration testing expert with advanced knowledge in web, recon, discovery & infrastructure penetration testing and blockchain protocols.
- Improve the security of the application by testing it both as white and black-box approaches
- Identify potential security issues that could be affecting the web application
In summary, Halborn identified one medium, one low and one informative security issues.
The Pink Elements assessed source code contained multiple vulnerable dependencies, some of them related to critical public-known vulnerabilities. Additionally, there was a lack of error handling on most of the parts of the source code, missing size and format checks of the input data. This could lead into uncontrolled behaviors during the application flow.
It is recommended to resolve all the security issues listed in the document to improve the security health of the application and its underlying infrastructure.
Also, it was possible to state that token vesting and locking according to the code will match the Pink Elements tokenomics.
Halborn performed a combination of manual and automated security testing to balance efficiency, timeliness, practicality, and accuracy in regard to the scope of this assessment. While manual testing is recommended to uncover flaws in logic, process and implementation; automated testing techniques help enhance coverage of the code and can quickly identify items that do not follow security best practices.
Several tests were carried out during the assessment; including, but not limited to:
- Vulnerable or outdated software or dependencies
- Brute force protections
- Access Handling
- Input Handling
- Fuzzing of all input parameters
- Sensitive information disclosure
- Application Logic Flaws
- Ensure that coding best practices are being followed by Pink Elements team
- Identify other potential vulnerabilities that may pose a risk to Pink Elements
Pink Elements team affirmed that they only stored in JSON files in cleartext sensitive information (such as KeyPairs, including Private Keys and Seed Words) for testing purposes.
The scoped source code that was assessed during the tests contained multiple secrets in cleartext. Like the following examples shows:
./accounts/admin.json
{ "publicKey": "hP6GA7iQFHw...[REDACTED]...ghvbLGPSvsjondV", "secretKey": "4udiX9...re14b...[REDACTED]...9vGvSo4uBgv3JBXVy" }
./modules/pinkToken/token.ts
constructor( adminSecret: string, connection: Connection, mintAccountPubKey: string | undefined, ) { //mint- and freezeAuthority and payer shall be set to admin this.admin = Keypair.fromSecretKey(base58.serialize(adminSecret)); this.connection = connection;
However, Halborn decided not to include this issue in the report because according to Pink Elements team:
This KeyPair is used only for test. We'll keep it in repo as it's just dummy wallets. Real wallets (admin and token distribution wallets) will be put to the VM (only access will be mine) with code. The main purpose of the storing KeyPairs (specially Secret Keys) in repo was just to simplify testing.
Otherwise, this must be considered a critical issue.
Critical
0
High
0
Medium
1
Low
1
Informational
1
Impact x Likelihood
HAL-01
HAL-02
HAL-03
| Security analysis | Risk level | Remediation Date |
|---|---|---|
| VULNERABLE THIRD-PARTY DEPENDENCIES | Medium | Partially Solved |
| UNHANDLED ERRORS | Low | Solved - 04/22/2024 |
| LACK OF INPUT FORMAT VALIDATION | Informational | Acknowledged |
// Medium
The scoped repositories used multiple third-party dependencies. Using vulnerable third-party libraries may result in security vulnerabilities in the project that can be exploited by attackers. This could result in data breaches, theft of sensitive information, and other security issues. However, some of them were affected by public-known vulnerabilities that may pose a risk to the global application security level.
In the image below, it is possible to observe the output from the npm audit command, showing some vulnerable package dependencies.
Besides, in the following pictures, it is possible to observe the output from yarn audit command, showing more vulnerable package dependencies.
Finally, also the snyk test command revealed multiple vulnerable dependencies, some of them with high and critical severity.
Update all affected packages to its latest version.
It is recommended to perform an automated analysis of the dependencies from the birth of the project and if they contain any security issues. Developers should be aware of this and apply any necessary mitigation measures to protect the affected application.
PARTIALLY SOLVED: The Pink Elements team partially addressed this issue on main/2128e085894e8683c865df47bb40ebaa6bafb0fe commit ID.
lordhenry@Pavel-GE626QF-Dragon:/mnt/f/Devel/Pink-Elements/Test/Halborn/solana-ts-pink-token$ yarn audit
yarn audit v1.22.22
warning package.json: License should be a valid SPDX license expression
info No lockfile found.
warning pink-elements-token@0.2.0: License should be a valid SPDX license expression
warning @metaplex-foundation/mpl-bubblegum > @metaplex-foundation/digital-asset-standard-api > package.json@2.0.1: Use pkg.json instead.
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ moderate │ Cross site scripting in parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=6.0.1 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @metaplex-foundation/mpl-bubblegum │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ @metaplex-foundation/mpl-bubblegum > │
│ │ @metaplex-foundation/digital-asset-standard-api > │
│ │ package.json > git-source > git-url-parse > git-up > │
│ │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://www.npmjs.com/advisories/1088917 │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ critical │ Server-Side Request Forgery in parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=6.0.1 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @metaplex-foundation/mpl-bubblegum │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ @metaplex-foundation/mpl-bubblegum > │
│ │ @metaplex-foundation/digital-asset-standard-api > │
│ │ package.json > git-source > git-url-parse > git-up > │
│ │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://www.npmjs.com/advisories/1088918 │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ moderate │ Cross site scripting in parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=6.0.1 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @metaplex-foundation/mpl-bubblegum │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ @metaplex-foundation/mpl-bubblegum > │
│ │ @metaplex-foundation/digital-asset-standard-api > │
│ │ package.json > git-source > git-url-parse > git-up > │
│ │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://www.npmjs.com/advisories/1088919 │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high │ Hostname confusion in parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=6.0.1 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @metaplex-foundation/mpl-bubblegum │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ @metaplex-foundation/mpl-bubblegum > │
│ │ @metaplex-foundation/digital-asset-standard-api > │
│ │ package.json > git-source > git-url-parse > git-up > │
│ │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://www.npmjs.com/advisories/1088920 │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ moderate │ parse-url parses http URLs incorrectly, making it vulnerable │
│ │ to host name spoofing │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=8.1.0 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @metaplex-foundation/mpl-bubblegum │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ @metaplex-foundation/mpl-bubblegum > │
│ │ @metaplex-foundation/digital-asset-standard-api > │
│ │ package.json > git-source > git-url-parse > git-up > │
│ │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://www.npmjs.com/advisories/1089114 │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ critical │ Server-Side Request Forgery (SSRF) in GitHub repository │
│ │ ionicabizau/parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=8.1.0 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @metaplex-foundation/mpl-bubblegum │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ @metaplex-foundation/mpl-bubblegum > │
│ │ @metaplex-foundation/digital-asset-standard-api > │
│ │ package.json > git-source > git-url-parse > git-up > │
│ │ parse-url │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://www.npmjs.com/advisories/1092304 │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ moderate │ Got allows a redirect to a UNIX socket │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ got │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=11.8.5 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @metaplex-foundation/mpl-bubblegum │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ @metaplex-foundation/mpl-bubblegum > │
│ │ @metaplex-foundation/digital-asset-standard-api > │
│ │ package.json > package-json > got │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://www.npmjs.com/advisories/1088948 │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high │ gry vulnerable to Command Injection │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ gry │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=6.0.0 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @metaplex-foundation/mpl-bubblegum │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ @metaplex-foundation/mpl-bubblegum > │
│ │ @metaplex-foundation/digital-asset-standard-api > │
│ │ package.json > git-package-json > gry │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://www.npmjs.com/advisories/1094428 │
└───────────────┴──────────────────────────────────────────────────────────────┘
8 vulnerabilities found - Packages audited: 333
Severity: 4 Moderate | 2 High | 2 Critical
Done in 20.78s.// Low
The code lacked of error handling, especially for blockchain transactions, which can fail for a variety of reasons. This might lead to unhandled exceptions and potentially leave transactions in an incomplete state. Particularly, the assessed source code lacked error handling in network operations like funding accounts,
generating keys, and checking balances. Without proper error catch blocks or validations, this could lead to unexpected and uncontrolled application execution flows, or even unintended data exposure through log prints.
intro here
Metadata.ts
The error handling in the createTokenMetadata method simply logged errors to the console. This approach may result in uncontrolled behavior of the execution flow when the method is called.
./modules/pinkToken/metadata.ts
try {
const txHash = await sendAndConfirmTransaction(
this.connection,
transaction,
[this.admin],
);
return txHash;
} catch (e) {
console.error('ERROR: Cannot create metadata for token');
console.error(e);
}Mint.ts: There was not return statement in case of an error occurs in all the methods
./modules/pinkToken/mint.ts
createNewToken = async () => {
try {
const mint = await createMint(
this.connection,
this.admin,
this.admin.publicKey,
this.admin.publicKey,
6, // changing to 6 to match default SPL tokens on Solana
);
const mintAccountPubKey = mint.toBase58();
this.mintAccount = new PublicKey(mintAccountPubKey);
return mintAccountPubKey;
} catch (e) {
console.error('Could not deploy new token');
console.error(e);
}
};Vesting.ts: No return statement when catching error could cause unhandled behavior.
./modules/pinkToken/vesting.ts
try {
const builtTx = new Transaction();
builtTx.feePayer = sourceAccount.publicKey;
builtTx.add(...tokenVestInstruction);
const tx = await sendAndConfirmTransaction(this.connection, builtTx, [
sourceAccount,
]);
return {
txhash: tx,
vestingAccountPubKey: this._getVestingAccountPubKey(seedWord),
};
} catch (e) {
console.error('ERROR: Could not send vesting transaction');
console.error(e);
}
};If code above throws an error (e.g. sendAndConfirmTransaction fails on the on-chain side), there is no execution flow handling on the Vesting.createNewVestProgram call, for example within the investorsDistro.ts file.
./test/tokenInit/investorsDistro.ts
const vestTx = await pinkToken.Vesting.createNewVestProgram(
publicKey,
vestingSeedWord,
vestSchedule,
undefined,
);
console.log(`created vesting for ${publicKey} for ${amount.toString()}`);
console.log('txHash', vestTx);The execution flow with no handled error cases mentioned above as an example, occurs on most of the methods in the scoped source code.
Accounts.ts
Also, in the accounts.ts file, there is not even an error handling control with a try-catch statement to return a different value or handle the potential error that may come up.
./modules/account.ts
let tx;
if (useAirdrop) {
tx = await this.connection.requestAirdrop(pubKey, 2 * LAMPORTS_PER_SOL);
} else {
const transaction = new Transaction().add(
web3.SystemProgram.transfer({
fromPubkey: signer.publicKey,
toPubkey: pubKey,
lamports: amount,
}),
);
transaction.feePayer = signer.publicKey;
tx = await sendAndConfirmTransaction(this.connection, transaction, [
signer,
]);
}
// Wait for airdrop confirmation
const confirm = await this.connection.confirmTransaction(tx);
return confirm;
};All the examples mentioned above may cause uncontrolled application behavior, undercover errors or inconsistent state of the execution flow. The following picture represents an example:
Also, related with the (HAL-03) LACK OF INPUT FORMAT VALIDATION, there were unhandled errors that may cause a inconsistent status of the execution flow.
For example, as the addresses format was not being checked, uncontrolled behavior of the application may occur during the execution flow, as shown in the next picture.
Considering implementing a more sophisticated error handling mechanism that can respond differently based on the error type.
Implement comprehensive error handling to manage transaction failures gracefully and offer resilient execution paths.
Replace console.log with actual error handling mechanisms. Return errors from functions and use try/catch blocks where exceptions might occur to handle errors elegantly at runtime.
SOLVED: Pink Elements team addressed this issue on main/2128e085894e8683c865df47bb40ebaa6bafb0fe commit ID. Listed below some examples of implemented changes.
./modules/pinkToken/metadata.ts
return txHash;
} catch (e) {
console.error('ERROR: Cannot create metadata for token');
console.error(e);
return null
}./modules/pinkToken/mint.ts
try {
const tx = await transfer(
this.connection,
fromWallet,
fromTokenAccount.address,
toTokenAccount.address,
fromWallet.publicKey,
amount,
);
return tx;
} catch (e) {
console.error(
`Could not transfer ${
amount * 10 ** -9
} from ${fromWallet.publicKey.toBase58()}to ${toPublicKey}`,
);
console.error(e);
return null;
}
};// Informational
There was no evident validation on inputs such as public keys, secret keys, and seed words for format, length, or legitimacy, which could lead to errors or vulnerabilities if malformed or malicious inputs are used.
Validation ensures that only correctly formatted and genuine data is processed, preventing a wide range of attacks.
For this particular case, the severity of this issue has been lowered because Pink Elements team acknowledged that only them will have access to view, modify or execute the assessed source code in scope. Thus, this issue lowers more to a Coding Best Practice, as there were no usual or external customers that were accessing and running the code, and all the "user-controlled" inputs should be legit and provided by the Pink Elements team during the runtime.
N/A
Implement rigorous input validation checks on all inputs, ensuring the format and size.
ACKNOWLEDGED: The Pink Elements team acknowledged this informational issue.
Halborn strongly recommends conducting a follow-up assessment of the project either within six months or immediately following any material changes to the codebase, whichever comes first. This approach is crucial for maintaining the project’s integrity and addressing potential vulnerabilities introduced by code modifications.
// Download the full report
* Use Google Chrome for best results
** Check "Background Graphics" in the print settings if needed