diff --git a/lib/ipv4/ipv4_packet.ml b/lib/ipv4/ipv4_packet.ml index d3ba8e479..03a345d60 100644 --- 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 index cddf8a8c8..b89ea7957 100644 --- 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 index e181b15a5..46f014c07 100644 --- 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 b/lib_test/test_ipv4.ml similarity index 84% rename from lib_test/test_ip_options.ml rename to lib_test/test_ipv4.ml index 3ba62fd6d..4314b291d 100644 --- a/lib_test/test_ip_options.ml +++ b/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 index 59c8a7ddf..6d21c688b 100644 --- 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;