reject unknown request fields
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
c0fb9974f7fe7f893b6dbeda8813a51e59a26d7b- Parents
-
01cdf42 - Tree
50e1ab1
c0fb997
c0fb9974f7fe7f893b6dbeda8813a51e59a26d7b01cdf42
50e1ab1| Status | File | + | - |
|---|---|---|---|
| M |
garwarp-ipc/src/lib.rs
|
21 | 0 |
| M |
garwarp/src/daemon.rs
|
29 | 0 |
garwarp-ipc/src/lib.rsmodified@@ -150,11 +150,17 @@ impl ControlRequest { | ||
| 150 | 150 | match parts.next() { |
| 151 | 151 | Some("inspect") => { |
| 152 | 152 | let fields = parse_fields(parts)?; |
| 153 | + if !fields_only(&fields, &["id"]) { | |
| 154 | + return None; | |
| 155 | + } | |
| 153 | 156 | let id = fields.get("id")?.clone(); |
| 154 | 157 | Some(Self::InspectRequest { id }) |
| 155 | 158 | } |
| 156 | 159 | Some("begin") => { |
| 157 | 160 | let fields = parse_fields(parts)?; |
| 161 | + if !fields_only(&fields, &["id", "sender", "app_id", "parent"]) { | |
| 162 | + return None; | |
| 163 | + } | |
| 158 | 164 | let id = fields.get("id")?.clone(); |
| 159 | 165 | let sender = fields.get("sender")?.clone(); |
| 160 | 166 | let app_id = fields.get("app_id").cloned(); |
@@ -168,6 +174,9 @@ impl ControlRequest { | ||
| 168 | 174 | } |
| 169 | 175 | Some("transition") => { |
| 170 | 176 | let fields = parse_fields(parts)?; |
| 177 | + if !fields_only(&fields, &["id", "sender", "state", "app_id"]) { | |
| 178 | + return None; | |
| 179 | + } | |
| 171 | 180 | let id = fields.get("id")?.clone(); |
| 172 | 181 | let sender = fields.get("sender")?.clone(); |
| 173 | 182 | let app_id = fields.get("app_id").cloned(); |
@@ -471,6 +480,12 @@ where | ||
| 471 | 480 | Some(fields) |
| 472 | 481 | } |
| 473 | 482 | |
| 483 | +fn fields_only(fields: &std::collections::HashMap<String, String>, allowed: &[&str]) -> bool { | |
| 484 | + fields | |
| 485 | + .keys() | |
| 486 | + .all(|key| allowed.iter().any(|allowed| key == allowed)) | |
| 487 | +} | |
| 488 | + | |
| 474 | 489 | #[cfg(test)] |
| 475 | 490 | mod tests { |
| 476 | 491 | use super::{ |
@@ -567,4 +582,10 @@ mod tests { | ||
| 567 | 582 | let parsed = ControlRequest::parse_line("inspect id=req-1 id=req-2"); |
| 568 | 583 | assert_eq!(parsed, None); |
| 569 | 584 | } |
| 585 | + | |
| 586 | + #[test] | |
| 587 | + fn request_parse_rejects_unknown_fields() { | |
| 588 | + let parsed = ControlRequest::parse_line("inspect id=req-1 bogus=1"); | |
| 589 | + assert_eq!(parsed, None); | |
| 590 | + } | |
| 570 | 591 | } |
garwarp/src/daemon.rsmodified@@ -524,6 +524,35 @@ mod tests { | ||
| 524 | 524 | ); |
| 525 | 525 | } |
| 526 | 526 | |
| 527 | + #[test] | |
| 528 | + fn unknown_request_fields_map_to_invalid_request() { | |
| 529 | + let (mut client, server) = UnixStream::pair().expect("pair should be created"); | |
| 530 | + client | |
| 531 | + .write_all(b"begin id=req-1 sender=:1.2 bogus=1\n") | |
| 532 | + .expect("begin request should be written"); | |
| 533 | + | |
| 534 | + let mut state = DaemonState { | |
| 535 | + health: HealthStatus::Healthy, | |
| 536 | + requests: RequestRegistry::new(Duration::from_secs(5)), | |
| 537 | + running: true, | |
| 538 | + }; | |
| 539 | + handle_connection(server, &mut state).expect("request should be handled"); | |
| 540 | + | |
| 541 | + let mut response_line = String::new(); | |
| 542 | + let mut reader = BufReader::new(client); | |
| 543 | + reader | |
| 544 | + .read_line(&mut response_line) | |
| 545 | + .expect("response should be readable"); | |
| 546 | + let response = ControlResponse::parse_line(&response_line).expect("response should parse"); | |
| 547 | + assert_eq!( | |
| 548 | + response, | |
| 549 | + ControlResponse::Error { | |
| 550 | + code: 2, | |
| 551 | + reason: "invalid_request".to_string(), | |
| 552 | + } | |
| 553 | + ); | |
| 554 | + } | |
| 555 | + | |
| 527 | 556 | #[test] |
| 528 | 557 | fn begin_request_tracks_parent_window_context() { |
| 529 | 558 | let (mut client, server) = UnixStream::pair().expect("pair should be created"); |