Merge pull request #127 from clecat/master

Check for output overflows in the non-streamable implementation

lib/de.ml

@@ -1525,25 +1525,6 @@ module Inf = struct
             unsafe_set_uint8 dst (dst_off + i) v
           done
 
-    let _fill v dst dst_off len =
-      let len0 = len land 3 in
-      let len1 = len asr 2 in
-
-      let nv = Nativeint.of_int v in
-      let vv = Nativeint.(logor (shift_left nv 8) nv) in
-      let vvvv = Nativeint.(logor (shift_left vv 16) vv) in
-      let vvvv = Nativeint.to_int32 vvvv in
-
-      for i = 0 to len1 - 1 do
-        let i = i * 4 in
-        unsafe_set_uint32 dst (dst_off + i) vvvv
-      done
-
-      ; for i = 0 to len0 - 1 do
-          let i = (len1 * 4) + i in
-          unsafe_set_uint8 dst (dst_off + i) v
-        done
-
     let flat d =
       d.i_pos <- d.i_pos - (d.bits / 8)
       ; d.hold <- 0
@@ -1608,7 +1589,8 @@ module Inf = struct
             d.hold <- d.hold lsr len
             ; d.bits <- d.bits - len
             ; if value < 256 then (
-                unsafe_set_uint8 d.o d.o_pos value
+                if d.o_pos >= d.o_len then err_unexpected_end_of_output ()
+                ; unsafe_set_uint8 d.o d.o_pos value
                 ; d.o_pos <- d.o_pos + 1
                 ; inflate_loop ())
               else if value == 256 then raise_notrace End
@@ -1633,11 +1615,12 @@ module Inf = struct
                         if d_ == 0 then raise_notrace Invalid_distance_code
                         ; if d_ > min d.o_pos (1 lsl 15) then
                             raise_notrace Invalid_distance
-                        ; let len = min l (d.o_len - d.o_pos) in
-                          let off = d.o_pos - d_ in
-                          _blit d.o off d.o d.o_pos len
-                          ; d.o_pos <- d.o_pos + len
-                          ; if l - len == 0 then inflate_loop () in
+                        ; let off = d.o_pos - d_ in
+                          if l > d.o_len - d.o_pos then
+                            err_unexpected_end_of_output ()
+                          ; _blit d.o off d.o d.o_pos l
+                          ; d.o_pos <- d.o_pos + l
+                          ; inflate_loop () in
         inflate_loop ()
       with
       | End -> ()
@@ -3236,16 +3219,18 @@ module Def = struct
       os.hold <- os.hold lor (bits lsl os.bits)
       ; os.bits <- os.bits + num_bits
       ; if os.bits >= 16 then begin
-          unsafe_set_uint16 os.o os.o_pos os.hold
-          ; if os.o_pos <> os.o_len then os.o_pos <- os.o_pos + 2
+          if os.o_pos + 1 >= os.o_len then err_unexpected_end_of_output ()
+          ; unsafe_set_uint16 os.o os.o_pos os.hold
+          ; os.o_pos <- os.o_pos + 2
           ; os.bits <- os.bits - 16
           ; os.hold <- os.hold lsr 16
         end
 
     let flush_bits os =
       if os.bits >= 8 then begin
-        unsafe_set_uint8 os.o os.o_pos os.hold
-        ; if os.o_pos <> os.o_len then os.o_pos <- os.o_pos + 1
+        if os.o_pos >= os.o_len then err_unexpected_end_of_output ()
+        ; unsafe_set_uint8 os.o os.o_pos os.hold
+        ; os.o_pos <- os.o_pos + 1
         ; os.bits <- os.bits - 8
         ; os.hold <- os.hold lsr 8
       end
@@ -3272,7 +3257,6 @@ module Def = struct
         ; incr i
         ; os.o_pos <- os.o_pos + 1
       done
-      ; os.i_pos <- os.i_pos + !i
 
     let write_uncompressed_block os len is_final_block =
       write_block_header os is_final_block blocktype_uncompressed
@@ -3292,13 +3276,13 @@ module Def = struct
         ; write_uncompressed_blocks os block_length is_final_block
 
     let flush_output os =
-      if os.o_pos == os.o_len then err_unexpected_end_of_output ()
-      ; while os.bits > 0 do
-          unsafe_set_uint8 os.o os.o_pos os.hold
-          ; os.o_pos <- os.o_pos + 1
-          ; os.bits <- os.bits - 8
-          ; os.hold <- os.hold lsr 8
-        done
+      while os.bits > 0 do
+        if os.o_pos >= os.o_len then err_unexpected_end_of_output ()
+        ; unsafe_set_uint8 os.o os.o_pos os.hold
+        ; os.o_pos <- os.o_pos + 1
+        ; os.bits <- os.bits - 8
+        ; os.hold <- os.hold lsr 8
+      done
       ; os.o_pos
 
     let compress_none _c i o =

lib/zl.ml

@@ -593,18 +593,24 @@ module Def = struct
       unsafe_set_uint16_be dst 0 header
 
     let deflate ?(level = 4) src dst =
-      header dst level
-      ; let sub_dst = bigstring_sub dst 2 (bigstring_length dst - 2) in
-        let res = De.Def.Ns.deflate ~level src sub_dst in
-        match res with
-        | Ok res ->
-          let adl32 =
-            Checkseum.Adler32.(
-              unsafe_digest_bigstring src 0 (bigstring_length src) default)
-          in
-          unsafe_set_uint32_be sub_dst res (Optint.to_int32 adl32)
-          ; Ok (res + 6)
-        | Error e -> Error (e : De.Def.Ns.error :> [> error ])
+      if bigstring_length dst < 2 then Error `Unexpected_end_of_output
+      else begin
+        header dst level
+        ; let sub_dst = bigstring_sub dst 2 (bigstring_length dst - 2) in
+          let res = De.Def.Ns.deflate ~level src sub_dst in
+          match res with
+          | Ok res ->
+            let adl32 =
+              Checkseum.Adler32.(
+                unsafe_digest_bigstring src 0 (bigstring_length src) default)
+            in
+            if bigstring_length sub_dst - res < 2 then
+              Error `Unexpected_end_of_output
+            else (
+              unsafe_set_uint32_be sub_dst res (Optint.to_int32 adl32)
+              ; Ok (res + 6))
+          | Error e -> Error (e : De.Def.Ns.error :> [> error ])
+      end
   end
 end
 

test/test_ns.ml

@@ -224,6 +224,49 @@ let invalid_distance_too_far_back () =
     (Error `Invalid_distance)
     (Inf.Ns.inflate src dst)
 
+let invalid_flat_not_enough_output () =
+  Alcotest.test_case "invalid output with flat block" `Quick @@ fun () ->
+  let src = bigstring_of_string "\x01\x04\x00\xfb\xff\xde\xad\xbe\xef" in
+  let dst = bigstring_create 0 in
+  Alcotest.(check check_decode)
+    "invalid distance too far back"
+    (Error `Unexpected_end_of_output)
+    (Inf.Ns.inflate src dst)
+
+let invalid_literal_not_enough_output () =
+  Alcotest.test_case "invalid output with literal" `Quick @@ fun () ->
+  let q = Queue.of_list [`Literal 'a'; `End] in
+  let b = Buffer.create 16 in
+  let encoder = Def.encoder (`Buffer b) ~q in
+  let go = function
+    | `Ok -> Buffer.contents b
+    | `Partial -> assert false
+    | `Block -> assert false in
+  let res = go (Def.encode encoder (`Block {Def.kind= Def.Fixed; last= true})) in
+  let src = bigstring_of_string res in
+  let dst = bigstring_create 0 in
+  Alcotest.(check check_decode)
+    "invalid distance too far back"
+    (Error `Unexpected_end_of_output)
+    (Inf.Ns.inflate src dst)
+
+let invalid_copy_not_enough_output () =
+  Alcotest.test_case "invalid output with copy" `Quick @@ fun () ->
+  let q = Queue.of_list [`Literal 'a'; `Copy (1, 3); `End] in
+  let b = Buffer.create 16 in
+  let encoder = Def.encoder (`Buffer b) ~q in
+  let go = function
+    | `Ok -> Buffer.contents b
+    | `Partial -> assert false
+    | `Block -> assert false in
+  let res = go (Def.encode encoder (`Block {Def.kind= Def.Fixed; last= true})) in
+  let src = bigstring_of_string res in
+  let dst = bigstring_create 1 in
+  Alcotest.(check check_decode)
+    "invalid distance too far back"
+    (Error `Unexpected_end_of_output)
+    (Inf.Ns.inflate src dst)
+
 let fixed () =
   Alcotest.test_case "fixed" `Quick @@ fun () ->
   let src = bigstring_of_string "\x03\x00" in
@@ -1162,16 +1205,13 @@ let test_corpus_with_zlib filename =
 
 let encoder_0 () =
   Alcotest.test_case "encoder 0" `Quick @@ fun () ->
-  let src = bigstring_of_string "\x00" in
+  let src = bigstring_of_string "\x01" in
   let res = Def.Ns.deflate ~level:0 src dst in
-  let expected = "\x01\x01\x00\xfe\xff\x00" in
+  let expected = "\x01\x01\x00\xfe\xff\x01" in
   Alcotest.(check (result int Alcotest.reject))
     "encoder 0"
     (Ok (String.length expected))
     res
-  ; ( Bigstringaf.to_string dst |> fun s ->
-      String.sub s 0 (check_encode res)
-      |> String.iteri (fun i -> Fmt.pr "%d: %C\n%!" i) )
   ; Alcotest.(check string)
       "0x00" expected
       (Bigstringaf.substring dst ~off:0 ~len:(check_encode res))
@@ -1194,9 +1234,6 @@ let encoder_1 () =
     "encoder 1"
     (Ok (String.length expected))
     res
-  ; ( Bigstringaf.to_string dst |> fun s ->
-      String.sub s 0 (check_encode res)
-      |> String.iteri (fun i j -> Fmt.pr "%d: \\x%02x\n%!" i (Char.code j)) )
   ; Alcotest.(check string)
       "0x00" expected
       (Bigstringaf.substring dst ~off:0 ~len:(check_encode res))
@@ -1210,7 +1247,8 @@ let tests =
       ; invalid_code_lengths (); invalid_bit_length_repeat (); invalid_codes ()
       ; invalid_lengths (); invalid_distances ()
       ; too_many_length_or_distance_symbols (); invalid_distance_code ()
-      ; invalid_distance_too_far_back ()
+      ; invalid_distance_too_far_back (); invalid_flat_not_enough_output ()
+      ; invalid_literal_not_enough_output (); invalid_copy_not_enough_output ()
       ] )
   ; ( "ns_valids"
     , [