T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/htlc.js:85
- Finding
- Trade Workflow Releases the Payment Secret Without Enforcing Asset Delivery<![CDATA[ ## Vulnerability Details **File Location**: `scripts/htlc.js:85-108` **Vulnerability Type**: Non-atomic asset-for-payment settlement **Risk Level**: Critical ### Vulnerable Code ```js // Full trade workflow async function trade(seller, inscriptionTx, ethAmount) { // Generate preimage const { preimage, hash } = generatePreimage(); const timeout = 3600; // 1 hour const lockHash = getLockHash(hash, seller, timeout); console.log('=== HTLC Trade ==='); console.log('Inscription:', inscriptionTx); console.log('Irys: https://gateway.irys.xyz/' + inscriptionTx); console.log('Preimage (keep secret):', preimage); console.log('PreimageHash:', hash); console.log('LockHash:', lockHash); // Lock funds await lock(seller, hash, timeout, ethAmount); console.log('\n=== Share with seller ==='); console.log('LockHash:', lockHash); console.log('Preimage:', preimage); return { lockHash, preimage, hash }; } ``` ### Technical Analysis The `trade` workflow accepts an `inscriptionTx` argument, but only prints it and constructs an Irys gateway URL. It does not: - Validate the transaction identifier. - Confirm that the advertised inscription or NFT exists. - Verify that the seller owns the asset. - Verify transfer of the asset to the buyer. - Escrow the asset in the same contract as the ETH. - Cryptographically bind the asset transfer to release of the ETH. After locking the buyer's ETH, the workflow prints the preimage and explicitly instructs the user to share it with the seller. Possession of that preimage enables the seller to invoke `reveal()` and release the locked funds, regardless of whether the asset was delivered. The preimage is also printed before the lock transaction is submitted. Terminal logs, automation logs, CI output, agent transcripts, or monitoring systems could therefore expose it prematurely. This is not an atomic exchange of an NFT or inscription for ETH. Only the payment side is represented in the impl ...[truncated 1191 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Do not disclose or print the preimage before enforceable asset-delivery conditions have been satisfied. 2. Implement a settlement contract that escrows both the ETH and the NFT or other transferable asset, releasing both sides atomically. 3. For assets that cannot be escrowed on the same chain, implement a separately audited and cryptographically verifiable cross-chain protocol rather than relying on an unverified transaction identifier. 4. Validate the asset contract, token ID, chain ID, current owner, approved transfer conditions, buyer address, and final transfer receipt. 5. Bind the payment lock to immutable trade parameters, including the asset contract, token ID, buyer, seller, chain ID, amount, and expiry. 6. Remove secrets from console output and return them only through a deliberately secured channel. 7. Warn users that shell history, process output, AI-agent transcripts, and CI logs are inappropriate secret-storage channels. 8. Remove the “atomic” and “trustless” claims until the asset and payment legs are technically coupled and independently audited. 9. Add adversarial integration tests proving that a seller cannot receive ETH without transferring the exact agreed asset. ]]>