Skip to content

Commit f9942ee

Browse files
committed
fix(node): validate ref-update SHAs are hex, not just length 40
parse_ref_updates accepted any 40-byte string as an object id, so a peer-supplied pkt-line with non-hex content was forwarded into RefUpdateEvents on gossip, branch_cid rows, and sync-notify JSON as a canonical-looking identifier. Require [0-9a-fA-F] on both fields. Closes #398
1 parent bfc44f9 commit f9942ee

1 file changed

Lines changed: 46 additions & 1 deletion

File tree

‎crates/gitlawb-node/src/api/repos.rs‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3153,6 +3153,12 @@ struct RefUpdate {
31533153
ref_name: String,
31543154
}
31553155

3156+
/// A git object id is 40 hex chars; length alone is not enough, since a peer
3157+
/// can send 40 arbitrary bytes (#398).
3158+
fn is_hex_sha(s: &str) -> bool {
3159+
s.len() == 40 && s.bytes().all(|b| b.is_ascii_hexdigit())
3160+
}
3161+
31563162
/// Parse git receive-pack pkt-line ref updates from the request body.
31573163
/// Format per line: `<40-hex-old> <40-hex-new> <refname>[NUL capabilities]\n`
31583164
fn parse_ref_updates(body: &[u8]) -> Vec<RefUpdate> {
@@ -3194,7 +3200,7 @@ fn parse_ref_updates(body: &[u8]) -> Vec<RefUpdate> {
31943200
.trim_end_matches('\n');
31953201

31963202
let parts: Vec<&str> = line.splitn(3, ' ').collect();
3197-
if parts.len() == 3 && parts[0].len() == 40 && parts[1].len() == 40 {
3203+
if parts.len() == 3 && is_hex_sha(parts[0]) && is_hex_sha(parts[1]) {
31983204
updates.push(RefUpdate {
31993205
old_sha: parts[0].to_string(),
32003206
new_sha: parts[1].to_string(),
@@ -3488,6 +3494,45 @@ mod tests {
34883494
);
34893495
}
34903496

3497+
// #398: a peer-supplied pkt-line whose 40-byte "SHA" fields carry non-hex
3498+
// content must be dropped, not forwarded into RefUpdate events, branch_cid
3499+
// rows, and sync-notify JSON as a canonical-looking identifier.
3500+
#[test]
3501+
fn parse_ref_updates_requires_hex_shas() {
3502+
let sha_a = "a".repeat(40);
3503+
let sha_b = "b".repeat(40);
3504+
let pkt = |old: &str, new: &str, refname: &str| {
3505+
let line = format!("{old} {new} {refname}");
3506+
format!("{:04x}{}0000", line.len() + 4, line).into_bytes()
3507+
};
3508+
3509+
// Valid 40-hex old + new -> accepted.
3510+
let updates = parse_ref_updates(&pkt(&sha_a, &sha_b, "refs/heads/main"));
3511+
assert_eq!(updates.len(), 1);
3512+
assert_eq!(updates[0].old_sha, sha_a);
3513+
assert_eq!(updates[0].new_sha, sha_b);
3514+
assert_eq!(updates[0].ref_name, "refs/heads/main");
3515+
3516+
// 40-byte non-hex fields -> dropped.
3517+
let junk = "Z".repeat(40);
3518+
assert!(parse_ref_updates(&pkt(&junk, &sha_b, "refs/heads/main")).is_empty());
3519+
assert!(parse_ref_updates(&pkt(&sha_a, &junk, "refs/heads/main")).is_empty());
3520+
3521+
// Wrong lengths -> dropped.
3522+
assert!(parse_ref_updates(&pkt(&"a".repeat(39), &sha_b, "refs/heads/main")).is_empty());
3523+
assert!(parse_ref_updates(&pkt(&sha_a, &"b".repeat(41), "refs/heads/main")).is_empty());
3524+
3525+
// A bad line among good ones drops only the bad line.
3526+
let sha_c = "c".repeat(40);
3527+
let bad = pkt(&junk, &sha_b, "refs/heads/bad");
3528+
let good = pkt(&sha_a, &sha_c, "refs/heads/good");
3529+
let mut mixed = bad[..bad.len() - 4].to_vec(); // strip trailing flush
3530+
mixed.extend_from_slice(&good);
3531+
let updates = parse_ref_updates(&mixed);
3532+
assert_eq!(updates.len(), 1);
3533+
assert_eq!(updates[0].ref_name, "refs/heads/good");
3534+
}
3535+
34913536
#[test]
34923537
fn git_service_app_error_classifies_timeout_bad_request_and_git() {
34933538
// GitServiceTimeout carried through anyhow -> 504 Timeout.

0 commit comments

Comments
 (0)