[New Snap] StarkNet

[kenhkan]

Checklist

All items in the list below needs to be satisfied.

• Is the Snap repository publicly accessible and linked in this ticket? https://github.com/ConsenSys/starknet-snap
• Is the Snap distributed on npm and linked in this ticket? https://www.npmjs.com/package/@consensys/starknet-snap
• Has an audit been performed and the audit report attached or linked in this issue? [New Snap] StarkNet #123 (comment)
• Is a complete list of discovered vulnerabilities from the audit documented in this issue? [New Snap] StarkNet #123 (comment)
• For vulnerabilities that have been deemed necessary to be addressed, are the links to the fixes attached to this issue? [New Snap] StarkNet #123 (comment)
• For vulnerabilities that have been deemed not necessary to be addressed, is a reason for each of them documented in this issue? [New Snap] StarkNet #123 (comment)
• The corresponding pull request in this repo has been merged. Update registry #50

[kenhkan]

Audit report:

https://consensys.io/diligence/audits/2023/06/metamask/partner-snaps-starknetsnap/

[Montoya]

List of findings with fixes or reasoning:

4.1 RPC starkNet_sendTransaction - The User Displayed Message Generated With getSigningTxnText() Is Prone to Markdown/Control Chars Injection From contractCallData

Fixed in https://github.com/Consensys/starknet-snap/tree/7231bb7fa4671283b2e7b4cbf5a519d56a57697a

4.2 Lax Validation Using@starknet::validateAndParseAddress Allows Short Addresses and Does Not Verify Checksums

Fixed in https://github.com/Consensys/starknet-snap/tree/7231bb7fa4671283b2e7b4cbf5a519d56a57697a

4.3 RPC starkNet_signMessage - Fails to Display the User Account That Is Used for Signing the Message

Fixed in https://github.com/Consensys/starknet-snap/tree/7231bb7fa4671283b2e7b4cbf5a519d56a57697a

4.4 RPC starkNet_signMessage - Inconsistency When Previewing the Signed Message (Markdown Injection)

Fixed in https://github.com/Consensys/starknet-snap/tree/7231bb7fa4671283b2e7b4cbf5a519d56a57697a

4.5 UI/AlertView - Unnecessary Use of dangerouslySetInnerHTML

Fixed in https://github.com/Consensys/starknet-snap/tree/7231bb7fa4671283b2e7b4cbf5a519d56a57697a

4.6 RPC starkNet_addErc20Token - Should Ask for User Confirmation

Fixed in https://github.com/Consensys/starknet-snap/tree/7231bb7fa4671283b2e7b4cbf5a519d56a57697a

4.7 getKeysFromAddress - Possible Unchecked Null Dereference When Looking Up Private Key

Fixed in https://github.com/Consensys/starknet-snap/tree/7231bb7fa4671283b2e7b4cbf5a519d56a57697a

4.8 RPC starkNet_getStoredTransactions - Lax or Missing Input Validation

This method is read only and therefore not needing input validation

4.9 Disable Debug Log for Production Build

Fixed in https://github.com/Consensys/starknet-snap/tree/7231bb7fa4671283b2e7b4cbf5a519d56a57697a

4.10 package.json - Dependency Mixup

Fixed in https://github.com/Consensys/starknet-snap/tree/7231bb7fa4671283b2e7b4cbf5a519d56a57697a

4.11 package.json - Invalid License

This finding is incorrect. Dual license is a valid approach.

4.12 RPC starkNet_extractPrivateKey - Should Be Renamed to starkNet_displayPrivateKey

This is a minor issue and does not need to be fixed.

4.13 UI/hooks - detectEthereumProvider() Should Require mustBeMetaMask

This is a minor issue and does not need to be fixed.

4.14 RPC starkNet_addNetwork - Not Implemented, No User Confirmation

This is a minor issue and does not need to be fixed.