Merge pull request #307 from yomimono/moar-parse-bugs

Test & fix more parse problems
mirage · Mar 23, 2017 · 6bd82bc · 6bd82bc
2 parents dc1795d + 2d77d05
commit 6bd82bc
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 7 deletions.
diff --git a/lib/ipv4/ipv4_packet.ml b/lib/ipv4/ipv4_packet.ml
@@ -113,6 +113,8 @@ module Unmarshal = struct
                           (get_ipv4_len buf) hlen)
         else if hlen < sizeof_ipv4 then Result.Error
           (Printf.sprintf "IPv4 header claimed to have size < 20: %d" hlen)
+        else if Cstruct.len buf < hlen then Result.Error
+          (Printf.sprintf "IPv4 packet w/length %d claimed to have header of size %d" (Cstruct.len buf) hlen)
         else Result.Ok hlen
     in
     let parse buf options_end =

diff --git a/lib/udp/udp_packet.ml b/lib/udp/udp_packet.ml
@@ -25,7 +25,7 @@ module Unmarshal = struct
         Error "UDP header claimed a total length < the size of just the header"
       else begin
         let payload_len = length_from_header - sizeof_udp in
-        if payload_len > length_of_buffer
+        if payload_len > (length_of_buffer - sizeof_udp)
         then Error (Printf.sprintf
 	      "UDP header claimed a payload longer than the supplied buffer: %d vs %d."
               payload_len length_of_buffer)

diff --git a/lib_test/test.ml b/lib_test/test.ml
@@ -16,17 +16,17 @@
 
 let suite = [
   "checksums"      , Test_checksums.suite   ;
-  "tcp_window"     , Test_tcp_window.suite  ;
-  "udp"            , Test_udp.suite         ;
-  "socket"         , Test_socket.suite      ;
+  "arp"            , Test_arp.suite         ;
+  "ipv4"           , Test_ipv4.suite        ;
+  "ipv6"           , Test_ipv6.suite        ;
   "icmpv4"         , Test_icmpv4.suite      ;
+  "udp"            , Test_udp.suite         ;
+  "tcp_window"     , Test_tcp_window.suite  ;
   "tcp_options"    , Test_tcp_options.suite ;
-  "ip_options"     , Test_ip_options.suite  ;
   "rfc5961"        , Test_rfc5961.suite     ;
-  "arp"            , Test_arp.suite         ;
+  "socket"         , Test_socket.suite      ;
   "connect"        , Test_connect.suite     ;
   "iperf"          , Test_iperf.suite       ;
-  "ipv6"           , Test_ipv6.suite        ;
 ]
 
 let run test () =

diff --git a/lib_test/test_ip_options.ml → lib_test/test_ipv4.ml b/lib_test/test_ip_options.ml → lib_test/test_ipv4.ml
@@ -27,6 +27,13 @@ let test_unmarshal_without_options () =
   | _ ->
       Alcotest.fail "Fail to parse ip packet with options"
 
+let test_unmarshal_regression () =
+  let p = Cstruct.of_string "\x49\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30\x30" in
+  Alcotest.(check (result reject pass))
+    "correctly return error for bad packet"
+    (Error "any") (Ipv4_packet.Unmarshal.of_cstruct p);
+  Lwt.return_unit
+
 let test_size () =
   let src = Ipaddr.V4.of_string_exn "127.0.0.1" in
   let dst = Ipaddr.V4.of_string_exn "127.0.0.2" in
@@ -42,5 +49,6 @@ let test_size () =
 let suite = [
   "unmarshal ip datagram with options", `Quick, test_unmarshal_with_options;
   "unmarshal ip datagram without options", `Quick, test_unmarshal_without_options;
+  "unmarshal ip datagram with no payload & hlen > 5", `Quick, test_unmarshal_regression;
   "size", `Quick, test_size;
 ]
diff --git a/lib_test/test_udp.ml b/lib_test/test_udp.ml
@@ -60,6 +60,16 @@ let write () =
   Static_arp.add_entry stack.arp dst (Macaddr.of_string_exn "00:16:3e:ab:cd:ef");
   Udp.write ~src_port:1212 ~dst_port:21 ~dst stack.udp (Cstruct.of_string "MGET *") >|= Rresult.R.get_ok
 
+let unmarshal_regression () =
+  let i = Cstruct.create 1016 in
+  Cstruct.memset i 30; 
+  Cstruct.set_char i 4 '\x04';
+  Cstruct.set_char i 5 '\x00';
+  Alcotest.(check (result reject pass)) "correctly return error for bad packet"
+    (Error "parse failed") (Udp_packet.Unmarshal.of_cstruct i);
+  Lwt.return_unit
+
+
 let marshal_marshal () =
   let error_str = Alcotest.result Alcotest.reject Alcotest.string in
   let udp = {Udp_packet.src_port = 1; dst_port = 2} in
@@ -77,6 +87,7 @@ let marshal_marshal () =
   Lwt.return_unit
 
 let suite = [
+  "unmarshal regression", `Quick, unmarshal_regression;
   "marshal/marshal", `Quick, marshal_marshal;
   "marshal/unmarshal", `Quick, marshal_unmarshal;
   "write packets", `Quick, write;