Skip to content

security(crypto): exploit Shamir Secret Sharing vulnerability to reco...#7

Open
bakaxbaka wants to merge 1 commit into
mainfrom
fix/shamir-secret-sharing-vulnerability-der-claude
Open

security(crypto): exploit Shamir Secret Sharing vulnerability to reco...#7
bakaxbaka wants to merge 1 commit into
mainfrom
fix/shamir-secret-sharing-vulnerability-der-claude

Conversation

@bakaxbaka
Copy link
Copy Markdown
Owner

No description provided.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Owner Author

@bakaxbaka bakaxbaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d

Copy link
Copy Markdown
Owner Author

@bakaxbaka bakaxbaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 5 additional findings in Devin Review.

Open in Devin Review

Comment thread recover_v6.py
Comment on lines +181 to +194
for a0_0, a1_0, a2_0 in first_byte_polys:
# Try to build a consistent entropy using this a2 pattern
for a2_pattern in range(256):
if a2_pattern == 255: # Skip invalid coefficient
continue

secret_bytes = []
valid = True

for i in range(16):
byte_polys = find_valid_polynomials_byte(x1, y1_list[i], x2, y2_list[i])

# Find polynomial with matching a2
matching = [p for p in byte_polys if p[2] == a2_pattern]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Outer loop variables unused in recover_v6.py causing full brute force to repeat per first-byte candidate

The outer loop at line 181 unpacks a0_0, a1_0, a2_0 from first_byte_polys, but none of these variables are used in the loop body. The inner loop (line 183) independently iterates over all 255 a2_pattern values, and filtering at line 194 uses a2_pattern instead of a2_0. This means the entire inner brute force (255 iterations × 16 bytes × expensive BIP derivation) is redundantly repeated for each element in first_byte_polys (up to ~254 elements). Compare with recover_v5.py:198 which correctly uses a2_0 from the outer loop to constrain the search. The likely fix is to either remove the outer loop entirely or use a2_0 instead of introducing the a2_pattern inner loop.

Prompt for agents
In recover_v6.py, the outer loop on line 181 iterates over first_byte_polys unpacking a0_0, a1_0, a2_0, but none of these variables are used inside the loop body. The inner loop on line 183 independently iterates over all a2_pattern values from 0 to 254, making the outer loop completely redundant.

This causes the entire brute-force search (including expensive BIP derivation per candidate) to repeat N times where N = len(first_byte_polys), which can be up to ~254.

Compare with recover_v5.py which correctly uses a2_0 from the outer loop at line 198: matching = [c for c in candidates_i if c[2] == a2_0].

The fix is one of:
1. Remove the outer loop and just iterate over a2_pattern (0-254) once, OR
2. Use a2_0 from the outer loop instead of the independent a2_pattern inner loop (matching recover_v5.py's approach)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread recover_final.py
def gf_add(a, b): return a ^ b
def gf_sub(a, b): return a ^ b
def gf_mul(a, b): return 0 if a == 0 or b == 0 else EXP[LOG[a] + LOG[b]]
def gf_div(a, b): return 0 if a == 0 else EXP[(LOG[a] - LOG[b]) % 255]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 gf_div silently returns wrong result when b=0 instead of raising an error

In recover_final.py:34 (and identically in recover_mnemonic.py:50, recover_v2.py:39, recover_v3.py:40, debug_byte1.py:20, test_gf256.py:22), gf_div has no guard for b == 0: it computes EXP[(LOG[a] - LOG[0]) % 255]. Since LOG[0] is initialized to 0, this silently evaluates to EXP[LOG[a]] = a, returning a instead of raising a division-by-zero error. This is mathematically wrong—division by zero is undefined in GF(256). The pybtc versions (recover_v4.py:46, recover_v5.py:45, recover_v6.py:45) correctly raise ZeroDivisionError(). With the current hardcoded share indices (x1=3, x2=15, denominator x1^x2=12), this doesn't trigger, but any future caller with b=0 would get silently incorrect results.

Suggested change
def gf_div(a, b): return 0 if a == 0 else EXP[(LOG[a] - LOG[b]) % 255]
def gf_div(a, b):
if b == 0: raise ZeroDivisionError("Division by zero in GF(256)")
return 0 if a == 0 else EXP[(LOG[a] - LOG[b]) % 255]
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

1 participant