Skip to content

fix(signature): validate pubkey length and prefix byte #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
oritwoen wants to merge 2 commits into
base: main
Choose a base branch
from
+104 −23
Open

fix(signature): validate pubkey length and prefix byte #26

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f23e54
fix(signature): validate pubkey length and prefix byte
oritwoen Mar 15, 2026
285f2f5
[autofix.ci] apply automated fixes
autofix-ci[bot] Mar 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/attack/polynonce.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,11 @@ mod tests {
assert_eq!(degree, 2, "Elimination polynomial should be quadratic");
}

// Valid-length test pubkeys for SignatureInput parsing
const TEST_PK_C: &str = "02cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc";
const TEST_PK_D: &str = "02dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd";
const TEST_PK_E: &str = "03eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee";

fn make_4_consecutive_sigs(pubkey: &str) -> Vec<Signature> {
(1..=4)
.map(|i| {
Expand All @@ -464,7 +469,7 @@ mod tests {
let attack = PolynonceAttack::new(1);
assert_eq!(attack.min_signatures(), 4);

let sigs = make_4_consecutive_sigs("02abcdef");
let sigs = make_4_consecutive_sigs(TEST_PK_C);
let vulns = attack.detect(&sigs);
assert_eq!(vulns.len(), 1);
}
Expand All @@ -479,7 +484,7 @@ mod tests {
r: format!("{}", 100 + i),
s: format!("{}", 200 + i),
z: format!("{}", 300 + i),
pubkey: Some("02abcdef".to_string()),
pubkey: Some(TEST_PK_C.to_string()),
timestamp: Some(i as u64),
kp: None,
})
Expand Down Expand Up @@ -517,8 +522,8 @@ mod tests {
fn test_polynonce_multiple_pubkeys_separate_groups() {
let attack = PolynonceAttack::new(1);

let mut sigs = make_4_consecutive_sigs("02aaaaaa");
sigs.extend(make_4_consecutive_sigs("02bbbbbb"));
let mut sigs = make_4_consecutive_sigs(TEST_PK_D);
sigs.extend(make_4_consecutive_sigs(TEST_PK_E));

let vulns = attack.detect(&sigs);
assert_eq!(vulns.len(), 2);
Expand Down Expand Up @@ -549,7 +554,7 @@ mod tests {
let attack = PolynonceAttack::new(1);
let group = SignatureGroup {
r: sigs[0].r,
pubkey: Some("02abcdef".to_string()),
pubkey: Some(TEST_PK_C.to_string()),
signatures: sigs,
confidence: 1.0,
};
Expand Down
107 changes: 89 additions & 18 deletions src/signature.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,32 @@ fn validate_pubkey_hex(pubkey: &str) -> Result<()> {
if !pubkey.chars().all(|c| c.is_ascii_hexdigit()) {
anyhow::bail!("Invalid pubkey: must be hexadecimal");
}

match pubkey.len() {
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Mar 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P2: This only checks SEC1 length and prefix, so invalid curve points still pass and later get reported as "unrecoverable" instead of bad input.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/signature.rs, line 97:

<comment>This only checks SEC1 length and prefix, so invalid curve points still pass and later get reported as "unrecoverable" instead of bad input.</comment>

<file context>
@@ -93,6 +93,32 @@ fn validate_pubkey_hex(pubkey: &str) -> Result<()> {
         anyhow::bail!("Invalid pubkey: must be hexadecimal");
     }
+
+    match pubkey.len() {
+        66 => {
+            if !pubkey.starts_with("02") && !pubkey.starts_with("03") {
</file context>
Fix with Cubic

66 => {
if !pubkey.starts_with("02") && !pubkey.starts_with("03") {
anyhow::bail!(
"Invalid compressed pubkey: must start with 02 or 03, got {}",
&pubkey[..2]
);
}
}
130 => {
if !pubkey.starts_with("04") {
anyhow::bail!(
"Invalid uncompressed pubkey: must start with 04, got {}",
&pubkey[..2]
);
}
}
len => {
anyhow::bail!(
"Invalid pubkey length: expected 66 (compressed) or 130 (uncompressed) hex chars, got {}",
len
);
}
}

Ok(())
}

Expand Down Expand Up @@ -165,6 +191,10 @@ pub fn group_by_r_and_pubkey(sigs: &[Signature]) -> Vec<SignatureGroup> {
mod tests {
use super::*;

// Valid-length test pubkeys (02/03 prefix + 64 hex chars = 66 total)
const TEST_PK_A: &str = "02aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
const TEST_PK_B: &str = "03bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb";

#[test]
fn test_signature_input_parse_decimal() {
let input = SignatureInput {
Expand All @@ -188,15 +218,15 @@ mod tests {
r: "123".to_string(),
s: "456".to_string(),
z: "789".to_string(),
pubkey: Some("02abcdef".to_string()),
pubkey: Some(TEST_PK_A.to_string()),
timestamp: None,
kp: None,
};
let input2 = SignatureInput {
r: "123".to_string(),
s: "111".to_string(),
z: "222".to_string(),
pubkey: Some("02abcdef".to_string()),
pubkey: Some(TEST_PK_A.to_string()),
timestamp: None,
kp: None,
};
Expand Down Expand Up @@ -244,15 +274,15 @@ mod tests {
r: "123".to_string(),
s: "456".to_string(),
z: "789".to_string(),
pubkey: Some("02abcdef".to_string()),
pubkey: Some(TEST_PK_A.to_string()),
timestamp: None,
kp: None,
};
let input2 = SignatureInput {
r: "123".to_string(),
s: "111".to_string(),
z: "222".to_string(),
pubkey: Some("03fedcba".to_string()),
pubkey: Some(TEST_PK_B.to_string()),
timestamp: None,
kp: None,
};
Expand All @@ -270,15 +300,15 @@ mod tests {
r: "123".to_string(),
s: "456".to_string(),
z: "789".to_string(),
pubkey: Some("02ABCDEF".to_string()),
pubkey: Some("02AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA".to_string()),
timestamp: None,
kp: None,
};
let input2 = SignatureInput {
r: "123".to_string(),
s: "111".to_string(),
z: "222".to_string(),
pubkey: Some("02abcdef".to_string()),
pubkey: Some(TEST_PK_A.to_string()),
timestamp: None,
kp: None,
};
Expand All @@ -297,15 +327,15 @@ mod tests {
r: "123".to_string(),
s: "456".to_string(),
z: "789".to_string(),
pubkey: Some("0x02abcdef").map(str::to_string),
pubkey: Some("0x02aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").map(str::to_string),
timestamp: None,
kp: None,
};
let input2 = SignatureInput {
r: "123".to_string(),
s: "111".to_string(),
z: "222".to_string(),
pubkey: Some("02abcdef").map(str::to_string),
pubkey: Some(TEST_PK_A).map(str::to_string),
timestamp: None,
kp: None,
};
Expand Down Expand Up @@ -396,17 +426,16 @@ mod tests {
#[test]
fn test_group_by_pubkey_ordered() {
let sigs = vec![
make_sig(Some("02abcdef"), Some(3)),
make_sig(Some("02abcdef"), Some(1)),
make_sig(Some("02abcdef"), Some(2)),
make_sig(Some("03fedcba"), Some(1)),
make_sig(Some(TEST_PK_A), Some(3)),
make_sig(Some(TEST_PK_A), Some(1)),
make_sig(Some(TEST_PK_A), Some(2)),
make_sig(Some(TEST_PK_B), Some(1)),
];
let groups = group_by_pubkey_ordered(&sigs);
assert_eq!(groups.len(), 2);
// 02abcdef group should be sorted by timestamp: 1, 2, 3
let pk1_group = groups
.iter()
.find(|g| g.pubkey == Some("02abcdef".to_string()))
.find(|g| g.pubkey == Some(TEST_PK_A.to_string()))
.unwrap();
assert_eq!(pk1_group.signatures[0].timestamp, Some(1));
assert_eq!(pk1_group.signatures[1].timestamp, Some(2));
Expand All @@ -425,7 +454,7 @@ mod tests {
let sigs = vec![
make_sig(None, Some(2)),
make_sig(None, Some(1)),
make_sig(Some("02abcdef"), Some(1)),
make_sig(Some(TEST_PK_A), Some(1)),
];
let groups = group_by_pubkey_ordered(&sigs);
assert_eq!(groups.len(), 2);
Expand All @@ -443,9 +472,9 @@ mod tests {
#[test]
fn test_group_by_pubkey_ordered_none_timestamps_sort_first() {
let sigs = vec![
make_sig(Some("02abcdef"), Some(3)),
make_sig(Some("02abcdef"), None),
make_sig(Some("02abcdef"), Some(1)),
make_sig(Some(TEST_PK_A), Some(3)),
make_sig(Some(TEST_PK_A), None),
make_sig(Some(TEST_PK_A), Some(1)),
];
let groups = group_by_pubkey_ordered(&sigs);
assert_eq!(groups.len(), 1);
Expand All @@ -456,4 +485,46 @@ mod tests {
assert_eq!(group.signatures[1].timestamp, Some(1));
assert_eq!(group.signatures[2].timestamp, Some(3));
}

#[test]
fn test_validate_pubkey_rejects_short_hex() {
let input = SignatureInput {
r: "123".to_string(),
s: "456".to_string(),
z: "789".to_string(),
pubkey: Some("02abcdef".to_string()),
timestamp: None,
kp: None,
};
let err = Signature::try_from(input).unwrap_err();
assert!(err.to_string().contains("Invalid pubkey length"));
}

#[test]
fn test_validate_pubkey_rejects_wrong_prefix() {
// 66 hex chars but wrong prefix (05 instead of 02/03)
let input = SignatureInput {
r: "123".to_string(),
s: "456".to_string(),
z: "789".to_string(),
pubkey: Some("05aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string()),
timestamp: None,
kp: None,
};
let err = Signature::try_from(input).unwrap_err();
assert!(err.to_string().contains("Invalid compressed pubkey"));
}

#[test]
fn test_validate_pubkey_accepts_uncompressed() {
let input = SignatureInput {
r: "123".to_string(),
s: "456".to_string(),
z: "789".to_string(),
pubkey: Some(format!("04{}", "aa".repeat(64))),
timestamp: None,
kp: None,
};
assert!(Signature::try_from(input).is_ok());
}
}
Loading