gardesk/garwarp / 4b69bf1

Browse files

harden response field parsing

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
4b69bf127b24d3f9c52e9bef36d4868f7d378f08
Parents
8060762
Tree
80907b4

1 changed file

StatusFile+-
M garwarp-ipc/src/lib.rs 108 39
garwarp-ipc/src/lib.rsmodified
@@ -299,38 +299,43 @@ impl ControlResponse {
299299
                         .ok_or(ParseError::InvalidField(part.to_string()))?;
300300
                     match key {
301301
                         "protocol" => {
302
-                            protocol_version = Some(
303
-                                value
304
-                                    .parse::<u16>()
305
-                                    .map_err(|_| ParseError::InvalidField(part.to_string()))?,
306
-                            );
302
+                            let parsed = value
303
+                                .parse::<u16>()
304
+                                .map_err(|_| ParseError::InvalidField(part.to_string()))?;
305
+                            if protocol_version.replace(parsed).is_some() {
306
+                                return Err(ParseError::InvalidField(part.to_string()));
307
+                            }
307308
                         }
308309
                         "health" => {
309
-                            health = HealthStatus::parse(value);
310
-                            if health.is_none() {
310
+                            let parsed = HealthStatus::parse(value)
311
+                                .ok_or(ParseError::InvalidField(part.to_string()))?;
312
+                            if health.replace(parsed).is_some() {
311313
                                 return Err(ParseError::InvalidField(part.to_string()));
312314
                             }
313315
                         }
314316
                         "in_flight" => {
315
-                            in_flight_requests = Some(
316
-                                value
317
-                                    .parse::<usize>()
318
-                                    .map_err(|_| ParseError::InvalidField(part.to_string()))?,
319
-                            );
317
+                            let parsed = value
318
+                                .parse::<usize>()
319
+                                .map_err(|_| ParseError::InvalidField(part.to_string()))?;
320
+                            if in_flight_requests.replace(parsed).is_some() {
321
+                                return Err(ParseError::InvalidField(part.to_string()));
322
+                            }
320323
                         }
321324
                         "total" => {
322
-                            total_requests = Some(
323
-                                value
324
-                                    .parse::<usize>()
325
-                                    .map_err(|_| ParseError::InvalidField(part.to_string()))?,
326
-                            );
325
+                            let parsed = value
326
+                                .parse::<usize>()
327
+                                .map_err(|_| ParseError::InvalidField(part.to_string()))?;
328
+                            if total_requests.replace(parsed).is_some() {
329
+                                return Err(ParseError::InvalidField(part.to_string()));
330
+                            }
327331
                         }
328332
                         "terminal" => {
329
-                            terminal_requests = Some(
330
-                                value
331
-                                    .parse::<usize>()
332
-                                    .map_err(|_| ParseError::InvalidField(part.to_string()))?,
333
-                            );
333
+                            let parsed = value
334
+                                .parse::<usize>()
335
+                                .map_err(|_| ParseError::InvalidField(part.to_string()))?;
336
+                            if terminal_requests.replace(parsed).is_some() {
337
+                                return Err(ParseError::InvalidField(part.to_string()));
338
+                            }
334339
                         }
335340
                         _ => return Err(ParseError::InvalidField(part.to_string())),
336341
                     }
@@ -358,8 +363,16 @@ impl ControlResponse {
358363
                             .split_once('=')
359364
                             .ok_or(ParseError::InvalidField(part.to_string()))?;
360365
                         match key {
361
-                            "id" => id = Some(value.to_string()),
362
-                            "state" => state = Some(value.to_string()),
366
+                            "id" => {
367
+                                if id.replace(value.to_string()).is_some() {
368
+                                    return Err(ParseError::InvalidField(part.to_string()));
369
+                                }
370
+                            }
371
+                            "state" => {
372
+                                if state.replace(value.to_string()).is_some() {
373
+                                    return Err(ParseError::InvalidField(part.to_string()));
374
+                                }
375
+                            }
363376
                             _ => return Err(ParseError::InvalidField(part.to_string())),
364377
                         }
365378
                     }
@@ -379,15 +392,18 @@ impl ControlResponse {
379392
                         .ok_or(ParseError::InvalidField(part.to_string()))?;
380393
                     match key {
381394
                         "ids" => {
382
-                            if value == "-" {
383
-                                ids = Some(Vec::new());
395
+                            let parsed = if value == "-" {
396
+                                Vec::new()
384397
                             } else {
385398
                                 let parsed =
386399
                                     value.split(',').map(str::to_string).collect::<Vec<_>>();
387400
                                 if parsed.iter().any(|id| id.is_empty()) {
388401
                                     return Err(ParseError::InvalidField(part.to_string()));
389402
                                 }
390
-                                ids = Some(parsed);
403
+                                parsed
404
+                            };
405
+                            if ids.replace(parsed).is_some() {
406
+                                return Err(ParseError::InvalidField(part.to_string()));
391407
                             }
392408
                         }
393409
                         _ => return Err(ParseError::InvalidField(part.to_string())),
@@ -403,24 +419,50 @@ impl ControlResponse {
403419
                 let mut sender = None;
404420
                 let mut app_id = None;
405421
                 let mut parent_window = None;
422
+                let mut saw_app_id = false;
423
+                let mut saw_parent_window = false;
406424
 
407425
                 for part in parts {
408426
                     let (key, value) = part
409427
                         .split_once('=')
410428
                         .ok_or(ParseError::InvalidField(part.to_string()))?;
411429
                     match key {
412
-                        "id" => id = Some(value.to_string()),
413
-                        "state" => state = Some(value.to_string()),
414
-                        "sender" => sender = Some(value.to_string()),
430
+                        "id" => {
431
+                            if id.replace(value.to_string()).is_some() {
432
+                                return Err(ParseError::InvalidField(part.to_string()));
433
+                            }
434
+                        }
435
+                        "state" => {
436
+                            if state.replace(value.to_string()).is_some() {
437
+                                return Err(ParseError::InvalidField(part.to_string()));
438
+                            }
439
+                        }
440
+                        "sender" => {
441
+                            if sender.replace(value.to_string()).is_some() {
442
+                                return Err(ParseError::InvalidField(part.to_string()));
443
+                            }
444
+                        }
415445
                         "app_id" => {
416
-                            if value != "-" {
417
-                                app_id = Some(value.to_string());
446
+                            if saw_app_id {
447
+                                return Err(ParseError::InvalidField(part.to_string()));
418448
                             }
449
+                            saw_app_id = true;
450
+                            app_id = if value == "-" {
451
+                                None
452
+                            } else {
453
+                                Some(value.to_string())
454
+                            };
419455
                         }
420456
                         "parent" => {
421
-                            if value != "-" {
422
-                                parent_window = Some(value.to_string());
457
+                            if saw_parent_window {
458
+                                return Err(ParseError::InvalidField(part.to_string()));
423459
                             }
460
+                            saw_parent_window = true;
461
+                            parent_window = if value == "-" {
462
+                                None
463
+                            } else {
464
+                                Some(value.to_string())
465
+                            };
424466
                         }
425467
                         _ => return Err(ParseError::InvalidField(part.to_string())),
426468
                     }
@@ -447,12 +489,18 @@ impl ControlResponse {
447489
                             .ok_or(ParseError::InvalidField(field.to_string()))?;
448490
                         match key {
449491
                             "code" => {
450
-                                code =
451
-                                    Some(value.parse::<u32>().map_err(|_| {
452
-                                        ParseError::InvalidField(field.to_string())
453
-                                    })?);
492
+                                let parsed = value
493
+                                    .parse::<u32>()
494
+                                    .map_err(|_| ParseError::InvalidField(field.to_string()))?;
495
+                                if code.replace(parsed).is_some() {
496
+                                    return Err(ParseError::InvalidField(field.to_string()));
497
+                                }
498
+                            }
499
+                            "reason" => {
500
+                                if reason.replace(value.to_string()).is_some() {
501
+                                    return Err(ParseError::InvalidField(field.to_string()));
502
+                                }
454503
                             }
455
-                            "reason" => reason = Some(value.to_string()),
456504
                             _ => return Err(ParseError::InvalidField(field.to_string())),
457505
                         }
458506
                     }
@@ -619,4 +667,25 @@ mod tests {
619667
         let parsed = ControlRequest::parse_line("inspect id=req-1 bogus=1");
620668
         assert_eq!(parsed, None);
621669
     }
670
+
671
+    #[test]
672
+    fn response_parse_rejects_duplicate_fields() {
673
+        assert!(
674
+            ControlResponse::parse_line(
675
+                "status protocol=1 protocol=2 health=healthy in_flight=0 total=0 terminal=0",
676
+            )
677
+            .is_err()
678
+        );
679
+        assert!(
680
+            ControlResponse::parse_line("ack request id=req-1 id=req-2 state=pending").is_err()
681
+        );
682
+        assert!(ControlResponse::parse_line("list ids=req-1 ids=req-2").is_err());
683
+        assert!(
684
+            ControlResponse::parse_line(
685
+                "snapshot id=req-1 state=pending sender=:1.2 app_id=- app_id=org.test.App parent=-",
686
+            )
687
+            .is_err()
688
+        );
689
+        assert!(ControlResponse::parse_line("error code=2 reason=bad reason=worse").is_err());
690
+    }
622691
 }