Merge pull request 'security: fix high-severity findings from audit (closes #6)' (#7) from fix/high-severity-security into main

Reviewed-on: #7

src/background/index.js

@@ -30,7 +30,6 @@ const connectedSites = {};
 
 // Pending approval requests: { id: { origin, hostname, resolve } }
 const pendingApprovals = {};
-let nextApprovalId = 1;
 
 async function getState() {
     const result = await storageApi.get("autistmask");
@@ -127,7 +126,7 @@ function openApprovalWindow(id) {
 // Prefers the browser-action popup (anchored to toolbar, no macOS Space switch).
 function requestApproval(origin, hostname) {
     return new Promise((resolve) => {
-        const id = nextApprovalId++;
+        const id = crypto.randomUUID();
         pendingApprovals[id] = { origin, hostname, resolve };
 
         if (actionApi && typeof actionApi.openPopup === "function") {
@@ -152,7 +151,7 @@ function requestApproval(origin, hostname) {
 // Uses the toolbar popup only — no fallback window.
 function requestTxApproval(origin, hostname, txParams) {
     return new Promise((resolve) => {
-        const id = nextApprovalId++;
+        const id = crypto.randomUUID();
         pendingApprovals[id] = {
             origin,
             hostname,
@@ -184,7 +183,7 @@ function requestTxApproval(origin, hostname, txParams) {
 // popup URL is still set, so the user can click the toolbar icon to respond.
 function requestSignApproval(origin, hostname, signParams) {
     return new Promise((resolve) => {
-        const id = nextApprovalId++;
+        const id = crypto.randomUUID();
         pendingApprovals[id] = {
             origin,
             hostname,
@@ -216,7 +215,7 @@ function requestSignApproval(origin, hostname, signParams) {
 // popups naturally close on focus loss and the user can reopen them.
 runtime.onConnect.addListener((port) => {
     if (port.name.startsWith("approval:")) {
-        const id = parseInt(port.name.split(":")[1], 10);
+        const id = port.name.split(":")[1];
         port.onDisconnect.addListener(() => {
             const approval = pendingApprovals[id];
             if (approval) {
@@ -442,6 +441,13 @@ async function handleRpc(method, params, origin) {
                 ? { method, message: params[0], from: params[1] }
                 : { method, message: params[1], from: params[0] };
 
+        if (method === "eth_sign") {
+            signParams.dangerWarning =
+                "\u26a0\ufe0f DANGER: This site is requesting to sign a raw hash. " +
+                "This can be used to sign transactions that drain your funds. " +
+                "Only proceed if you fully understand what you are signing.";
+        }
+
         const decision = await requestSignApproval(
             origin,
             hostname,
@@ -611,12 +617,39 @@ if (windowsApi && windowsApi.onRemoved) {
 // Listen for messages from content scripts and popup
 runtime.onMessage.addListener((msg, sender, sendResponse) => {
     if (msg.type === "AUTISTMASK_RPC") {
-        handleRpc(msg.method, msg.params, msg.origin).then((response) => {
+        // Derive origin from trusted sender info to prevent origin spoofing.
+        // Chrome MV3 provides sender.origin; Firefox MV2 fallback uses sender.tab.url.
+        let trustedOrigin = msg.origin; // fallback only if sender info unavailable
+        if (sender.origin) {
+            trustedOrigin = sender.origin;
+        } else if (sender.tab && sender.tab.url) {
+            try {
+                trustedOrigin = new URL(sender.tab.url).origin;
+            } catch {
+                // keep fallback
+            }
+        }
+        handleRpc(msg.method, msg.params, trustedOrigin).then((response) => {
             sendResponse(response);
         });
         return true;
     }
 
+    // Validate that popup-only messages originate from the extension itself.
+    const POPUP_ONLY_TYPES = [
+        "AUTISTMASK_GET_APPROVAL",
+        "AUTISTMASK_APPROVAL_RESPONSE",
+        "AUTISTMASK_TX_RESPONSE",
+        "AUTISTMASK_SIGN_RESPONSE",
+    ];
+    if (POPUP_ONLY_TYPES.includes(msg.type)) {
+        const extUrl = runtime.getURL("");
+        if (!sender.url || !sender.url.startsWith(extUrl)) {
+            sendResponse({ error: "Unauthorized sender" });
+            return false;
+        }
+    }
+
     if (msg.type === "AUTISTMASK_GET_APPROVAL") {
         const approval = pendingApprovals[msg.id];
         if (approval) {
@@ -681,7 +714,8 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                     if (wallet) break;
                 }
                 if (!wallet) throw new Error("Wallet not found");
-                const decrypted = await decryptWithPassword(
+                // TODO(security): Move decryption to popup to avoid sending password via runtime.sendMessage
+                let decrypted = await decryptWithPassword(
                     wallet.encryptedSecret,
                     msg.password,
                 );
@@ -690,6 +724,10 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                     addrIndex,
                     decrypted,
                 );
+                // Best-effort: clear decrypted secret after use.
+                // Note: JS strings are immutable; this nulls the reference but
+                // the original string may persist in memory until GC.
+                decrypted = null;
                 const provider = getProvider(state.rpcUrl);
                 const connected = signer.connect(provider);
                 const tx = await connected.sendTransaction(approval.txParams);
@@ -735,7 +773,8 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                     if (wallet) break;
                 }
                 if (!wallet) throw new Error("Wallet not found");
-                const decrypted = await decryptWithPassword(
+                // TODO(security): Move decryption to popup to avoid sending password via runtime.sendMessage
+                let decrypted = await decryptWithPassword(
                     wallet.encryptedSecret,
                     msg.password,
                 );
@@ -744,6 +783,10 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                     addrIndex,
                     decrypted,
                 );
+                // Best-effort: clear decrypted secret after use.
+                // Note: JS strings are immutable; this nulls the reference but
+                // the original string may persist in memory until GC.
+                decrypted = null;
 
                 const sp = approval.signParams;
                 let signature;

src/popup/index.html

@@ -1015,6 +1015,17 @@
                     wants you to sign a message.
                 </p>
 
+                <div
+                    id="approve-sign-danger-warning"
+                    class="hidden mb-3 p-2 text-xs font-bold"
+                    style="
+                        background: #fee2e2;
+                        color: #991b1b;
+                        border: 2px solid #dc2626;
+                        border-radius: 6px;
+                    "
+                ></div>
+
                 <div class="mb-3">
                     <div class="text-xs text-muted mb-1">Type</div>
                     <div id="approve-sign-type" class="text-xs font-bold"></div>

src/popup/views/addWallet.js

@@ -49,8 +49,8 @@ function init(ctx) {
             showFlash("Please choose a password.");
             return;
         }
-        if (pw.length < 8) {
-            showFlash("Password must be at least 8 characters.");
+        if (pw.length < 12) {
+            showFlash("Password must be at least 12 characters.");
             return;
         }
         if (pw !== pw2) {

src/popup/views/approval.js

@@ -294,6 +294,18 @@ function showSignApproval(details) {
         }
     }
 
+    // Display danger warning for eth_sign (raw hash signing)
+    const warningEl = $("approve-sign-danger-warning");
+    if (warningEl) {
+        if (sp.dangerWarning) {
+            warningEl.textContent = sp.dangerWarning;
+            warningEl.classList.remove("hidden");
+        } else {
+            warningEl.textContent = "";
+            warningEl.classList.add("hidden");
+        }
+    }
+
     $("approve-sign-password").value = "";
     $("approve-sign-error").classList.add("hidden");
     $("btn-approve-sign").disabled = false;
@@ -373,6 +385,7 @@ function init(ctx) {
                 type: "AUTISTMASK_TX_RESPONSE",
                 id: approvalId,
                 approved: true,
+                // TODO(security): Move decryption to popup to avoid sending password via runtime.sendMessage
                 password: password,
             },
             (response) => {
@@ -412,6 +425,7 @@ function init(ctx) {
                 type: "AUTISTMASK_SIGN_RESPONSE",
                 id: approvalId,
                 approved: true,
+                // TODO(security): Move decryption to popup to avoid sending password via runtime.sendMessage
                 password: password,
             },
             (response) => {

src/popup/views/confirmTx.js

@@ -334,8 +334,13 @@ function init(ctx) {
                 tx = await contract.transfer(pendingTx.to, amount);
             }
 
+            // Best-effort: clear decrypted secret after use.
+            // Note: JS strings are immutable; this nulls the reference but
+            // the original string may persist in memory until GC.
+            decryptedSecret = null;
             txStatus.showWait(pendingTx, tx.hash);
         } catch (e) {
+            decryptedSecret = null;
             const hash = tx ? tx.hash : null;
             txStatus.showError(pendingTx, hash, e.shortMessage || e.message);
         }

src/popup/views/importKey.js

@@ -30,8 +30,8 @@ function init(ctx) {
             showFlash("Please choose a password.");
             return;
         }
-        if (pw.length < 8) {
-            showFlash("Password must be at least 8 characters.");
+        if (pw.length < 12) {
+            showFlash("Password must be at least 12 characters.");
             return;
         }
         if (pw !== pw2) {