Skip to content

fix(security): gate webhook creation through the public-host validator (#81) #92

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

Merged
kevincodex1 merged 1 commit into from
Jun 25, 2026
Merged

fix(security): gate webhook creation through the public-host validator (#81) #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
101 changes: 91 additions & 10 deletions crates/gitlawb-node/src/api/peers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,38 @@ pub struct PeerResponse {
pub reachable: bool,
}

/// Extract an IPv4 address embedded in an IPv6 literal across the transition
/// formats that carry one: IPv4-mapped (`::ffff:a.b.c.d`), IPv4-compatible
/// (`::a.b.c.d`), 6to4 (`2002:WWXX:YYZZ::/16`), and the NAT64 well-known prefix
/// (`64:ff9b::/96`). Returns `None` for native IPv6. Callers fold the result
/// back through the v4 range checks so loopback/private addresses smuggled in
/// via any of these encodings are rejected, not just the mapped/compatible pair.
///
/// INCOMPLETE-NAT64 CONTRACT: non-well-known NAT64 prefixes (e.g. the RFC 8215
/// local-use `64:ff9b:1::/48`) are deliberately **not** decoded here — their v4
/// sits at a prefix-length-dependent offset (RFC 6052 §2.2) — so they return
/// `None`. Any caller that needs them blocked must reject the wider
/// `64:ff9b::/32` itself; `is_public_http_url` does this in its native-v6 arm.
fn embedded_ipv4(v6: std::net::Ipv6Addr) -> Option<std::net::Ipv4Addr> {
if let Some(v4) = v6.to_ipv4_mapped().or_else(|| v6.to_ipv4()) {
return Some(v4);
}
let s = v6.segments();
// 6to4 (2002::/16) embeds W.X.Y.Z in the next two segments.
if s[0] == 0x2002 {
let [a, b] = s[1].to_be_bytes();
let [c, d] = s[2].to_be_bytes();
return Some(std::net::Ipv4Addr::new(a, b, c, d));
}
// NAT64 well-known prefix (64:ff9b::/96) embeds the IPv4 in the low 32 bits.
if s[0] == 0x0064 && s[1] == 0xff9b && s[2..6] == [0, 0, 0, 0] {
let [a, b] = s[6].to_be_bytes();
let [c, d] = s[7].to_be_bytes();
return Some(std::net::Ipv4Addr::new(a, b, c, d));
}
None
}

/// Whether a peer `http_url` is a public http(s) endpoint safe to register.
/// Rejects non-http(s) schemes, loopback/unspecified/private/link-local IPs,
/// and `localhost` / `.local` / `.internal` hostnames. Used at announce time
Expand Down Expand Up @@ -78,15 +110,12 @@ pub fn is_public_http_url(raw: &str) -> bool {
if ip.is_loopback() || ip.is_unspecified() {
return false;
}
// Fold IPv4-mapped/compatible IPv6 (`::ffff:127.0.0.1`, `::127.0.0.1`)
// down to IPv4 so the v4 range checks catch loopback/private addresses
// smuggled in via an IPv6 literal, then re-check loopback/unspecified.
// Fold any IPv6 literal that embeds an IPv4 address (mapped, compatible,
// 6to4, NAT64) down to that IPv4 so the v4 range checks catch
// loopback/private addresses smuggled in via an IPv6 encoding, then
// re-check loopback/unspecified.
let ip = match ip {
std::net::IpAddr::V6(v6) => v6
.to_ipv4_mapped()
.or_else(|| v6.to_ipv4())
.map(std::net::IpAddr::V4)
.unwrap_or(ip),
std::net::IpAddr::V6(v6) => embedded_ipv4(v6).map(std::net::IpAddr::V4).unwrap_or(ip),
v4 => v4,
};
if ip.is_loopback() || ip.is_unspecified() {
Expand All @@ -95,8 +124,14 @@ pub fn is_public_http_url(raw: &str) -> bool {
match ip {
std::net::IpAddr::V4(v4) => {
let o = v4.octets();
// RFC1918 private, link-local, or CGNAT (100.64.0.0/10).
if v4.is_private() || v4.is_link_local() || (o[0] == 100 && (o[1] & 0xc0) == 64) {
// RFC1918 private, link-local, CGNAT (100.64.0.0/10), or the
// RFC1122 "this host" block 0.0.0.0/8 (never a valid destination;
// 0.0.0.0 itself is already caught by the is_unspecified check).
if v4.is_private()
|| v4.is_link_local()
|| (o[0] == 100 && (o[1] & 0xc0) == 64)
|| o[0] == 0
{
return false;
}
}
Expand All @@ -106,6 +141,16 @@ pub fn is_public_http_url(raw: &str) -> bool {
if (s[0] & 0xfe00) == 0xfc00 || (s[0] & 0xffc0) == 0xfe80 {
return false;
}
// Any NAT64 address (64:ff9b::/32) that is not the cleanly
// decodable well-known /96 — e.g. the RFC 8215 local-use
// 64:ff9b:1::/48 — carries a translated target whose embedded v4
// sits at a prefix-length-dependent offset (RFC 6052 §2.2).
// Rather than risk a wrong decode across every prefix length we
// reject the whole NAT64 space here. The well-known /96 was
// already folded to its v4 above and never reaches this arm.
if s[0] == 0x0064 && s[1] == 0xff9b {
return false;
}
}
}
}
Expand Down Expand Up @@ -425,6 +470,11 @@ mod tests {
"http://127.0.0.1:5432/",
"http://localhost:22/",
"http://0.0.0.0:7545",
// RFC1122 "this host" block 0.0.0.0/8 beyond 0.0.0.0 itself,
// including the upper boundary (1.0.0.0 is public, tested below).
"http://0.0.0.1:7545",
"http://0.1.2.3/",
"http://0.255.255.255/",
"http://10.0.0.5:7545",
"http://192.168.1.10/",
"http://172.16.0.1:7545",
Expand All @@ -445,15 +495,46 @@ mod tests {
"http://[::ffff:10.0.0.1]/",
"http://[::ffff:192.168.1.1]:7545",
"http://[::127.0.0.1]/",
// 6to4 (2002::/16) embedding loopback/private v4.
"http://[2002:7f00:1::]:7545",
"http://[2002:a00:1::]/",
"http://[2002:c0a8:101::]/",
// NAT64 well-known prefix (64:ff9b::/96) embedding loopback/private v4.
"http://[64:ff9b::7f00:1]/",
"http://[64:ff9b::a00:1]:7545",
// NAT64 local-use prefix (64:ff9b:1::/48, RFC 8215) — rejected
// outright rather than decoded, so the whole NAT64 space is closed.
"http://[64:ff9b:1::7f00:1]/",
"http://[64:ff9b:1::a00:1]:7545",
"http://[64:ff9b:1:a00::]/",
// Local-use NAT64 wrapping a *public* v4 (203.0.113.10) is still
// rejected: the block is prefix-based, not payload-based.
"http://[64:ff9b:1::cb00:710a]/",
] {
assert!(!is_public_http_url(bad), "{bad:?} must be rejected");
}
}

#[test]
fn accepts_6to4_and_nat64_wrapping_public_v4() {
// Fold-and-recheck stays consistent with the mapped/compatible handling:
// a transition address that wraps a public v4 is allowed, only the
// private/loopback-wrapping forms above are rejected. 203.0.113.10.
assert!(is_public_http_url("http://[2002:cb00:710a::]/"));
assert!(is_public_http_url("http://[64:ff9b::cb00:710a]/"));
}

#[test]
fn accepts_public_outside_cgnat_range() {
// 100.0.0.0/10 boundary: .63 and .128 are public, only 100.64-127 is CGNAT.
assert!(is_public_http_url("http://100.63.255.255:7545"));
assert!(is_public_http_url("http://100.128.0.1/"));
}

#[test]
fn accepts_first_address_above_this_host_block() {
// 1.0.0.0 is the first address above the rejected 0.0.0.0/8 block;
// pins the `o[0] == 0` check against an off-by-one into 1.x.
assert!(is_public_http_url("http://1.0.0.0/"));
}
}
42 changes: 39 additions & 3 deletions crates/gitlawb-node/src/api/webhooks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,14 @@ pub async fn create_webhook(
.ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?;
crate::api::require_repo_owner(&record, &auth.0)?;

// Validate URL is http/https
if !req.url.starts_with("http://") && !req.url.starts_with("https://") {
// Gate the target through the same hardened public-host validator the peer
// announce path uses, so an authenticated owner cannot register a webhook
// that makes the node POST to loopback/private/link-local/metadata
// endpoints (SSRF). Delivery runs on the shared no-redirect client
// (main.rs), which closes the 3xx-to-internal bounce.
if !crate::api::peers::is_public_http_url(&req.url) {
return Err(AppError::BadRequest(
"webhook URL must be http:// or https://".into(),
"webhook URL must be a public http(s) URL (no loopback, private, or .internal/.local hosts)".into(),
));
}
Comment on lines +43 to 52

@coderabbitai coderabbitai Bot Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Locate webhook delivery code and inspect the reqwest client construction.
fd -t f -e rs | xargs rg -nP -C3 '(redirect|Policy::none|resolve\(|dns_resolver|deliver|webhook)' 2>/dev/null

# Specifically look for any post-resolution IP re-validation in delivery.
rg -nP -C4 'is_public_http_url|to_socket_addrs|lookup_host|resolve' crates/gitlawb-node/src 2>/dev/null

Repository: Gitlawb/node

Length of output: 50369

🏁 Script executed:

# Find is_public_http_url implementation to understand what it validates
rg -A20 'fn is_public_http_url' crates/gitlawb-node/src 2>/dev/null

# Check if there's any custom DNS resolver or socket filtering
rg -n 'dns_resolver|custom_resolver|socket_addr|to_socket_addrs|0\.0\.0\.0|127\.0\.0\.1|private_ip|is_private' crates/gitlawb-node/src/webhooks.rs 2>/dev/null

# Search for any IP validation in webhooks or related code
rg -n 'IpAddr|ip_addr|resolve|lookup' crates/gitlawb-node/src/webhooks.rs 2>/dev/null

Repository: Gitlawb/node

Length of output: 1661

🏁 Script executed:

# Get the complete is_public_http_url function
rg -A30 'fn is_public_http_url' crates/gitlawb-node/src/api/peers.rs

# Check the full webhooks.rs delivery code for any IP validation
cat -n crates/gitlawb-node/src/webhooks.rs | head -130

Repository: Gitlawb/node

Length of output: 5941

🏁 Script executed:

# Get the complete is_public_http_url function - need more lines
rg -A50 'fn is_public_http_url' crates/gitlawb-node/src/api/peers.rs | head -60

Repository: Gitlawb/node

Length of output: 2323

🏁 Script executed:

# Get the complete is_public_http_url function
rg -A80 'fn is_public_http_url' crates/gitlawb-node/src/api/peers.rs | head -100

Repository: Gitlawb/node

Length of output: 3548

Webhook delivery does not validate resolved IP addresses; DNS-rebinding SSRF remains exploitable.

is_public_http_url validates only the URL's hostname literals and suffix patterns (rejecting localhost, .local, .internal), and rejects IP literals in loopback/private ranges. However, a public hostname (e.g., webhook.attacker.example) that resolves to 127.0.0.1 or a private IP passes this gate, since DNS resolution happens at delivery time.

The webhook delivery code in crates/gitlawb-node/src/webhooks.rs line 85 (client.post(&hook.url)) performs no validation of the resolved address. The HTTP client (main.rs) uses Policy::none() to prevent 3xx redirects but has no custom DNS resolver, socket filter, or connect-time IP guards. Add IP address validation at delivery time to validate the resolved socket address matches the URL's validation requirements.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/gitlawb-node/src/api/webhooks.rs` around lines 43 - 52, The webhook
URL validation at registration time only checks the hostname string, not the
actual resolved IP address, which allows DNS-rebinding attacks where a public
hostname resolves to a loopback or private IP. Add IP address validation at
webhook delivery time in the `client.post(&hook.url)` call in
crates/gitlawb-node/src/webhooks.rs around line 85. Before posting the request,
resolve the webhook URL's hostname and validate that the resolved socket address
passes the same IP range checks as `is_public_http_url` (rejecting loopback and
private IP ranges). This ensures that even if DNS resolves to a private address
at delivery time, the request is blocked.


Expand Down Expand Up @@ -120,3 +124,35 @@ pub async fn delete_webhook(
state.db.delete_webhook(&id).await?;
Ok(Json(serde_json::json!({ "deleted": true, "id": id })))
}

#[cfg(test)]
mod tests {
use crate::api::peers::is_public_http_url;

// create_webhook gates req.url through is_public_http_url. Pin the exact
// SSRF targets from issue #81 so the webhook path can never regress to the
// old scheme-only check, independent of the validator's own peer tests.
#[test]
fn webhook_url_gate_rejects_ssrf_targets() {
for bad in [
"http://127.0.0.1:5432/",
"http://169.254.169.254/latest/meta-data/",
"http://localhost/",
"http://10.0.0.5/",
"http://[::1]/",
// IPv6 transition encodings smuggling loopback v4 (6to4 / NAT64).
"http://[2002:7f00:1::]/",
"http://[64:ff9b::7f00:1]/",
"ftp://example.com/",
"not-a-url",
] {
assert!(!is_public_http_url(bad), "{bad:?} must be rejected");
}
}

#[test]
fn webhook_url_gate_allows_public_targets() {
assert!(is_public_http_url("https://hooks.example.com/gitlawb"));
assert!(is_public_http_url("http://203.0.113.10:7545/"));
}
}
Loading