Skip to content

Merge fix/shamir-secret-sharing-vulnerability-der-blackbox into main#18

Open
bakaxbaka wants to merge 2 commits intomainfrom
fix/shamir-secret-sharing-vulnerability-der-blackbox
Open

Merge fix/shamir-secret-sharing-vulnerability-der-blackbox into main#18
bakaxbaka wants to merge 2 commits intomainfrom
fix/shamir-secret-sharing-vulnerability-der-blackbox

Conversation

@bakaxbaka
Copy link
Copy Markdown
Owner

Automated PR created from task completion

print(f'Share indices: x1={x1}, x2={x2}')
print(f'Derivation path: {path_name}')
if passphrase:
print(f'Passphrase: {passphrase}')

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.

Copilot Autofix

AI 5 months ago

To resolve the issue, the line that prints the passphrase (line 137) must be modified to avoid logging the sensitive value directly. There are several ways to fix this:

  • Remove the line: If displaying the passphrase is not required for functionality, simply remove the print statement.
  • Mask the passphrase value: If it's helpful to indicate that a passphrase was used, print that a passphrase was provided, or mask part/all of its value (e.g. show only the string "(provided)" or asterisks).

The best approach is to avoid printing the passphrase at all. Users who know what they entered do not need to see the passphrase echoed back. Therefore, in search, inside the successful recovery info block, remove or replace line 137:

  • If a passphrase is provided, print 'Passphrase: (provided)' or simply skip this line.

Required changes:

  • Edit recover.py line 137 (inside the successful recovery block in search). Do not print the passphrase.
    No new imports or method changes are needed; simply update this print statement.

Suggested changeset 1
recover.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/recover.py b/recover.py
--- a/recover.py
+++ b/recover.py
@@ -134,7 +134,7 @@
                     print(f'Share indices: x1={x1}, x2={x2}')
                     print(f'Derivation path: {path_name}')
                     if passphrase:
-                        print(f'Passphrase: {passphrase}')
+                        print(f'Passphrase: (provided)')
                     print(f'\nRecovered mnemonic:')
                     print(f'  {mnemonic}')
                     print(f'\n{"="*70}')
EOF
@@ -134,7 +134,7 @@
print(f'Share indices: x1={x1}, x2={x2}')
print(f'Derivation path: {path_name}')
if passphrase:
print(f'Passphrase: {passphrase}')
print(f'Passphrase: (provided)')
print(f'\nRecovered mnemonic:')
print(f' {mnemonic}')
print(f'\n{"="*70}')
Copilot is powered by AI and may make mistakes. Always verify output.
commit 371b730
Author: bakaxbaka <profesyonelceset@gmail.com>
Date:   Tue Oct 28 08:21:19 2025 +0300

    Guard recovery copy button when key missing

commit f9c7fec
Author: bakaxbaka <profesyonelceset@gmail.com>
Date:   Tue Oct 28 08:21:15 2025 +0300

    Fix copy button scope in recovery modal

commit 4fe111f
Author: bakaxbaka <profesyonelceset@gmail.com>
Date:   Tue Oct 28 07:59:40 2025 +0300

    Refine k-reuse recovery payload and UI
? addressEntries.map(([type, addr]) => `<div><strong>${type}:</strong> <code>${addr}</code></div>`).join('')
: '<em>No derived addresses available.</em>';
copyTarget = hasPrivateKeyWif ? privateKeyWifRaw : '';
copyTargetEscaped = copyTarget ? copyTarget.replace(/'/g, "\\'") : '';

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This does not escape backslash characters in the input.

Copilot Autofix

AI 5 months ago

The safest and most complete fix is to also escape backslashes in addition to single quotes. This is typically accomplished by first replacing all existing backslashes (\) with double backslashes (\\), and then replacing all single quotes (') with escaped single quotes (\'). The order of operations matters: you must escape backslashes before you escape single quotes to prevent double-escaping of newly added backslashes.

Changes should be made only to line 489 in static/app.js. Specifically, replace

copyTargetEscaped = copyTarget ? copyTarget.replace(/'/g, "\\'") : '';

with

copyTargetEscaped = copyTarget ? copyTarget.replace(/\\/g, '\\\\').replace(/'/g, "\\'") : '';

There is no need to import external libraries, since this can be safely handled with regular expressions.


Suggested changeset 1
static/app.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/static/app.js b/static/app.js
--- a/static/app.js
+++ b/static/app.js
@@ -486,7 +486,7 @@
             ? addressEntries.map(([type, addr]) => `<div><strong>${type}:</strong> <code>${addr}</code></div>`).join('')
             : '<em>No derived addresses available.</em>';
         copyTarget = hasPrivateKeyWif ? privateKeyWifRaw : '';
-        copyTargetEscaped = copyTarget ? copyTarget.replace(/'/g, "\\'") : '';
+        copyTargetEscaped = copyTarget ? copyTarget.replace(/\\/g, '\\\\').replace(/'/g, "\\'") : '';
 
         content = `
             <div class="mb-3">
EOF
@@ -486,7 +486,7 @@
? addressEntries.map(([type, addr]) => `<div><strong>${type}:</strong> <code>${addr}</code></div>`).join('')
: '<em>No derived addresses available.</em>';
copyTarget = hasPrivateKeyWif ? privateKeyWifRaw : '';
copyTargetEscaped = copyTarget ? copyTarget.replace(/'/g, "\\'") : '';
copyTargetEscaped = copyTarget ? copyTarget.replace(/\\/g, '\\\\').replace(/'/g, "\\'") : '';

content = `
<div class="mb-3">
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants