Skip to content

Commit 6044935

Browse files
congwang-mkclaude
andcommitted
fix(security): normalize HTTP paths before ACL matching
Add normalize_path() that decodes percent-encoding, collapses duplicate slashes, and resolves . and .. segments before matching against ACL rules. Without this, attackers could bypass deny rules via: /v1//admin/settings (double slash) /v1/../admin/settings (dot-dot traversal) /%61dmin/settings (percent-encoded 'a') Applied to both incoming request paths (in matches()) and rule paths (in parse()) so both sides are in canonical form. Includes 12 new unit tests covering normalization and bypass prevention. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1f6753b commit 6044935

1 file changed

Lines changed: 153 additions & 3 deletions

File tree

crates/sandlock-core/src/policy.rs

Lines changed: 153 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,13 @@ impl HttpRule {
101101

102102
let (host, path) = if let Some(pos) = rest.find('/') {
103103
let (h, p) = rest.split_at(pos);
104-
(h.to_string(), p.to_string())
104+
// Normalize the rule path, but preserve trailing * for glob matching.
105+
let has_wildcard = p.ends_with('*');
106+
let mut normalized = normalize_path(p);
107+
if has_wildcard && !normalized.ends_with('*') {
108+
normalized.push('*');
109+
}
110+
(h.to_string(), normalized)
105111
} else {
106112
(rest.to_string(), "/*".to_string())
107113
};
@@ -114,6 +120,8 @@ impl HttpRule {
114120
}
115121

116122
/// Check whether this rule matches the given request parameters.
123+
/// The request path is normalized before matching to prevent bypasses
124+
/// via `//`, `/../`, `/.`, or percent-encoding.
117125
pub fn matches(&self, method: &str, host: &str, path: &str) -> bool {
118126
// Method match
119127
if self.method != "*" && !self.method.eq_ignore_ascii_case(method) {
@@ -123,9 +131,63 @@ impl HttpRule {
123131
if self.host != "*" && !self.host.eq_ignore_ascii_case(host) {
124132
return false;
125133
}
126-
// Path match
127-
prefix_or_exact_match(&self.path, path)
134+
// Path match — normalize to prevent encoding/traversal bypasses
135+
let normalized = normalize_path(path);
136+
prefix_or_exact_match(&self.path, &normalized)
137+
}
138+
}
139+
140+
/// Normalize an HTTP path to prevent ACL bypasses via encoding tricks.
141+
///
142+
/// - Decodes percent-encoded characters (e.g. `%2F` → `/`, `%61` → `a`)
143+
/// - Collapses duplicate slashes (`//` → `/`)
144+
/// - Resolves `.` and `..` segments
145+
/// - Ensures the path starts with `/`
146+
pub fn normalize_path(path: &str) -> String {
147+
// 1. Percent-decode
148+
let mut decoded = String::with_capacity(path.len());
149+
let mut chars = path.bytes();
150+
while let Some(b) = chars.next() {
151+
if b == b'%' {
152+
let hi = chars.next();
153+
let lo = chars.next();
154+
if let (Some(h), Some(l)) = (hi, lo) {
155+
let hex = [h, l];
156+
if let Ok(s) = std::str::from_utf8(&hex) {
157+
if let Ok(val) = u8::from_str_radix(s, 16) {
158+
decoded.push(val as char);
159+
continue;
160+
}
161+
}
162+
// Malformed percent encoding — keep as-is
163+
decoded.push(b as char);
164+
decoded.push(h as char);
165+
decoded.push(l as char);
166+
} else {
167+
decoded.push(b as char);
168+
}
169+
} else {
170+
decoded.push(b as char);
171+
}
128172
}
173+
174+
// 2. Split into segments, resolve . and .., skip empty segments (collapses //)
175+
let mut segments: Vec<&str> = Vec::new();
176+
for seg in decoded.split('/') {
177+
match seg {
178+
"" | "." => {}
179+
".." => {
180+
segments.pop();
181+
}
182+
s => segments.push(s),
183+
}
184+
}
185+
186+
// 3. Reconstruct with leading /
187+
let mut result = String::with_capacity(decoded.len());
188+
result.push('/');
189+
result.push_str(&segments.join("/"));
190+
result
129191
}
130192

131193
/// Simple prefix or exact matching for paths. Supports trailing `*` as a prefix match.
@@ -886,4 +948,92 @@ mod http_rule_tests {
886948
assert!(policy.https_ca.is_some());
887949
assert!(policy.https_key.is_some());
888950
}
951+
952+
// --- normalize_path tests ---
953+
954+
#[test]
955+
fn normalize_path_basic() {
956+
assert_eq!(normalize_path("/foo/bar"), "/foo/bar");
957+
assert_eq!(normalize_path("/"), "/");
958+
}
959+
960+
#[test]
961+
fn normalize_path_double_slashes() {
962+
assert_eq!(normalize_path("/foo//bar"), "/foo/bar");
963+
assert_eq!(normalize_path("//foo///bar//"), "/foo/bar");
964+
}
965+
966+
#[test]
967+
fn normalize_path_dot_segments() {
968+
assert_eq!(normalize_path("/foo/./bar"), "/foo/bar");
969+
assert_eq!(normalize_path("/foo/../bar"), "/bar");
970+
assert_eq!(normalize_path("/foo/bar/../../baz"), "/baz");
971+
}
972+
973+
#[test]
974+
fn normalize_path_dotdot_at_root() {
975+
assert_eq!(normalize_path("/../foo"), "/foo");
976+
assert_eq!(normalize_path("/../../foo"), "/foo");
977+
}
978+
979+
#[test]
980+
fn normalize_path_percent_encoding() {
981+
// %2F = /, %61 = a
982+
assert_eq!(normalize_path("/foo%2Fbar"), "/foo/bar");
983+
assert_eq!(normalize_path("/%61dmin/settings"), "/admin/settings");
984+
}
985+
986+
#[test]
987+
fn normalize_path_mixed_bypass_attempts() {
988+
// Double-encoded traversal
989+
assert_eq!(normalize_path("/v1/./admin/settings"), "/v1/admin/settings");
990+
assert_eq!(normalize_path("/v1/../admin/settings"), "/admin/settings");
991+
assert_eq!(normalize_path("/v1//admin/settings"), "/v1/admin/settings");
992+
assert_eq!(normalize_path("/v1/%2e%2e/admin"), "/admin");
993+
}
994+
995+
// --- ACL bypass prevention tests ---
996+
997+
#[test]
998+
fn acl_deny_prevents_double_slash_bypass() {
999+
let deny = vec![HttpRule::parse("* */admin/*").unwrap()];
1000+
// These should all be caught by the deny rule
1001+
assert!(!http_acl_check(&[], &deny, "GET", "example.com", "/admin/settings"));
1002+
assert!(!http_acl_check(&[], &deny, "GET", "example.com", "//admin/settings"));
1003+
assert!(!http_acl_check(&[], &deny, "GET", "example.com", "/admin//settings"));
1004+
}
1005+
1006+
#[test]
1007+
fn acl_deny_prevents_dot_segment_bypass() {
1008+
let deny = vec![HttpRule::parse("* */admin/*").unwrap()];
1009+
assert!(!http_acl_check(&[], &deny, "GET", "example.com", "/./admin/settings"));
1010+
assert!(!http_acl_check(&[], &deny, "GET", "example.com", "/public/../admin/settings"));
1011+
}
1012+
1013+
#[test]
1014+
fn acl_deny_prevents_percent_encoding_bypass() {
1015+
let deny = vec![HttpRule::parse("* */admin/*").unwrap()];
1016+
// %61dmin = admin
1017+
assert!(!http_acl_check(&[], &deny, "GET", "example.com", "/%61dmin/settings"));
1018+
}
1019+
1020+
#[test]
1021+
fn acl_allow_normalized_path_still_works() {
1022+
let allow = vec![HttpRule::parse("GET example.com/v1/models").unwrap()];
1023+
assert!(http_acl_check(&allow, &[], "GET", "example.com", "/v1/models"));
1024+
assert!(http_acl_check(&allow, &[], "GET", "example.com", "/v1/./models"));
1025+
assert!(http_acl_check(&allow, &[], "GET", "example.com", "/v1//models"));
1026+
// These resolve to different paths and should be denied
1027+
assert!(!http_acl_check(&allow, &[], "GET", "example.com", "/v1/models/extra"));
1028+
assert!(!http_acl_check(&allow, &[], "GET", "example.com", "/v2/models"));
1029+
}
1030+
1031+
#[test]
1032+
fn parse_normalizes_rule_path() {
1033+
let rule = HttpRule::parse("GET example.com/v1/./models/*").unwrap();
1034+
assert_eq!(rule.path, "/v1/models/*");
1035+
1036+
let rule = HttpRule::parse("GET example.com/v1//models").unwrap();
1037+
assert_eq!(rule.path, "/v1/models");
1038+
}
8891039
}

0 commit comments

Comments
 (0)