Skip to content

Commit

Permalink
Add bounds checking for negative lenghts in blit operations
Browse files Browse the repository at this point in the history
  • Loading branch information
Antonio Nuno Monteiro committed Mar 11, 2019
1 parent 463984e commit bf75228
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 22 deletions.
25 changes: 20 additions & 5 deletions lib/bigstringaf.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,17 @@ let[@inline never] invalid_bounds_memcmp op buf1_len buf1_off buf2_len buf2_off

let copy t ~off ~len =
let buffer_len = length t in
if off < 0 || buffer_len - off < len then invalid_bounds "copy" buffer_len off len;
if len < 0 || off < 0 || buffer_len - off < len
then invalid_bounds "copy" buffer_len off len;
let dst = create len in
unsafe_blit t ~src_off:off dst ~dst_off:0 ~len;
dst
;;

let substring t ~off ~len =
let buffer_len = length t in
if off < 0 || buffer_len - off < len then invalid_bounds "substring" buffer_len off len;
if len < 0 || off < 0 || buffer_len - off < len
then invalid_bounds "substring" buffer_len off len;
let b = Bytes.create len in
unsafe_blit_to_bytes t ~src_off:off b ~dst_off:0 ~len;
Bytes.unsafe_to_string b
Expand All @@ -108,7 +110,8 @@ let to_string t =

let of_string ~off ~len s =
let buffer_len = String.length s in
if off < 0 || buffer_len - off < len then invalid_bounds "of_string" buffer_len off len;
if len < 0 || off < 0 || buffer_len - off < len
then invalid_bounds "of_string" buffer_len off len;
let b = create len in
unsafe_blit_from_string s ~src_off:off b ~dst_off:0 ~len;
b
Expand All @@ -117,6 +120,8 @@ let of_string ~off ~len s =
let blit src ~src_off dst ~dst_off ~len =
let src_len = length src in
let dst_len = length dst in
if len < 0
then invalid_bounds_blit "blit" src_len src_off dst_len dst_off len;
if src_off < 0 || src_len - src_off < len
then invalid_bounds_blit "blit" src_len src_off dst_len dst_off len;
if dst_off < 0 || dst_len - dst_off < len
Expand All @@ -127,6 +132,8 @@ let blit src ~src_off dst ~dst_off ~len =
let blit_from_string src ~src_off dst ~dst_off ~len =
let src_len = String.length src in
let dst_len = length dst in
if len < 0
then invalid_bounds_blit "blit_from_string" src_len src_off dst_len dst_off len;
if src_off < 0 || src_len - src_off < len
then invalid_bounds_blit "blit_from_string" src_len src_off dst_len dst_off len;
if dst_off < 0 || dst_len - dst_off < len
Expand All @@ -137,6 +144,8 @@ let blit_from_string src ~src_off dst ~dst_off ~len =
let blit_from_bytes src ~src_off dst ~dst_off ~len =
let src_len = Bytes.length src in
let dst_len = length dst in
if len < 0
then invalid_bounds_blit "blit_from_bytes" src_len src_off dst_len dst_off len;
if src_off < 0 || src_len - src_off < len
then invalid_bounds_blit "blit_from_bytes" src_len src_off dst_len dst_off len;
if dst_off < 0 || dst_len - dst_off < len
Expand All @@ -147,6 +156,8 @@ let blit_from_bytes src ~src_off dst ~dst_off ~len =
let blit_to_bytes src ~src_off dst ~dst_off ~len =
let src_len = length src in
let dst_len = Bytes.length dst in
if len < 0
then invalid_bounds_blit "blit_to_bytes" src_len src_off dst_len dst_off len;
if src_off < 0 || src_len - src_off < len
then invalid_bounds_blit "blit_to_bytes" src_len src_off dst_len dst_off len;
if dst_off < 0 || dst_len - dst_off < len
Expand All @@ -157,6 +168,8 @@ let blit_to_bytes src ~src_off dst ~dst_off ~len =
let memcmp buf1 buf1_off buf2 buf2_off len =
let buf1_len = length buf1 in
let buf2_len = length buf2 in
if len < 0
then invalid_bounds_memcmp "memcmp" buf1_len buf1_off buf2_len buf2_off len;
if buf1_off < 0 || buf1_len - buf1_off < len
then invalid_bounds_memcmp "memcmp" buf1_len buf1_off buf2_len buf2_off len;
if buf2_off < 0 || buf2_len - buf2_off < len
Expand All @@ -167,10 +180,12 @@ let memcmp buf1 buf1_off buf2 buf2_off len =
let memcmp_string buf1 buf1_off buf2 buf2_off len =
let buf1_len = length buf1 in
let buf2_len = String.length buf2 in
if len < 0
then invalid_bounds_memcmp "memcmp_string" buf1_len buf1_off buf2_len buf2_off len;
if buf1_off < 0 || buf1_len - buf1_off < len
then invalid_bounds_memcmp "memcmp" buf1_len buf1_off buf2_len buf2_off len;
then invalid_bounds_memcmp "memcmp_string" buf1_len buf1_off buf2_len buf2_off len;
if buf2_off < 0 || buf2_len - buf2_off < len
then invalid_bounds_memcmp "memcmp" buf1_len buf1_off buf2_len buf2_off len;
then invalid_bounds_memcmp "memcmp_string" buf1_len buf1_off buf2_len buf2_off len;
unsafe_memcmp_string buf1 buf1_off buf2 buf2_off len
;;

Expand Down
89 changes: 72 additions & 17 deletions lib_test/test_bigstringaf.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ let setters m () =
f buffer
in
let substring ~len buffer = Bigstringaf.substring ~off:0 ~len buffer in

with_buffer ~f:(fun buffer ->
set buffer 0 '\xde';
Alcotest.(check string "set" "\xde___" (substring ~len:4 buffer)));
Expand Down Expand Up @@ -233,6 +233,60 @@ let memcmp_string m () =
()
;;

let negative_bounds_check () =
let open Bigstringaf in
let buf = Bigstringaf.empty in
let exn_str fn =
Invalid_argument
(Printf.sprintf
"Bigstringaf.%s invalid range: { buffer_len: 0, off: 0, len: -8 }"
fn)
in
let exn_ba fn =
Invalid_argument
(Printf.sprintf
"Bigstringaf.%s invalid range: { src_len: 0, src_off: 0, dst_len: 0, dst_off: 4, len: -8 }"
fn)
in
let exn_cmp fn =
Invalid_argument
(Printf.sprintf
"Bigstringaf.%s invalid range: { buf1_len: 0, buf1_off: 0, buf2_len: 0, buf2_off: 0, len: -8 }"
fn)
in
Alcotest.check_raises "copy"
(exn_str "copy")
(fun () -> ignore (copy buf ~off:0 ~len:(-8)));
Alcotest.check_raises "substring"
(exn_str "substring")
(fun () -> ignore (substring buf ~off:0 ~len:(-8)));
Alcotest.check_raises "of_string"
(exn_str "of_string")
(fun () -> ignore (of_string "" ~off:0 ~len:(-8)));
Alcotest.check_raises "blit"
(exn_ba "blit")
(fun () -> ignore (blit buf ~src_off:0 buf ~dst_off:4 ~len:(-8)));
Alcotest.check_raises "blit_from_string"
(exn_ba "blit_from_string")
(fun () ->
ignore (blit_from_string "" ~src_off:0 buf ~dst_off:4 ~len:(-8)));
Alcotest.check_raises "blit_from_bytes"
(exn_ba "blit_from_bytes")
(fun () ->
ignore (blit_from_bytes (Bytes.of_string "") ~src_off:0 buf ~dst_off:4 ~len:(-8)));
Alcotest.check_raises "blit_to_bytes"
(exn_ba "blit_to_bytes")
(fun () ->
ignore (blit_to_bytes buf ~src_off:0 (Bytes.of_string "") ~dst_off:4 ~len:(-8)));
Alcotest.check_raises "memcmp"
(exn_cmp "memcmp")
(fun () ->
ignore (memcmp buf 0 buf 0 (-8)));
Alcotest.check_raises "memcmp_string"
(exn_cmp "memcmp_string")
(fun () ->
ignore (memcmp_string buf 0 "" 0 (-8)));
;;

let safe_operations =
let module Getters : S.Getters = Bigstringaf in
Expand All @@ -247,36 +301,37 @@ let safe_operations =
; "blit_from_bytes" , `Quick, blit_from_bytes (module Blit)
; "memcmp" , `Quick, memcmp (module Memcmp)
; "memcmp_string" , `Quick, memcmp_string (module Memcmp)
; "negative length" , `Quick, negative_bounds_check
]

let unsafe_operations =
let unsafe_operations =
let module Getters : S.Getters = struct
open Bigstringaf

let get = unsafe_get
let get = unsafe_get

let get_int16_le = unsafe_get_int16_le
let get_int16_sign_extended_le = unsafe_get_int16_sign_extended_le
let get_int32_le = unsafe_get_int32_le
let get_int64_le = unsafe_get_int64_le
let get_int16_le = unsafe_get_int16_le
let get_int16_sign_extended_le = unsafe_get_int16_sign_extended_le
let get_int32_le = unsafe_get_int32_le
let get_int64_le = unsafe_get_int64_le

let get_int16_be = unsafe_get_int16_be
let get_int16_sign_extended_be = unsafe_get_int16_sign_extended_be
let get_int32_be = unsafe_get_int32_be
let get_int16_be = unsafe_get_int16_be
let get_int16_sign_extended_be = unsafe_get_int16_sign_extended_be
let get_int32_be = unsafe_get_int32_be
let get_int64_be = unsafe_get_int64_be
end in
let module Setters : S.Setters = struct
open Bigstringaf

let set = unsafe_set
let set = unsafe_set

let set_int16_le = unsafe_set_int16_le
let set_int32_le = unsafe_set_int32_le
let set_int64_le = unsafe_set_int64_le
let set_int16_le = unsafe_set_int16_le
let set_int32_le = unsafe_set_int32_le
let set_int64_le = unsafe_set_int64_le

let set_int16_be = unsafe_set_int16_be
let set_int32_be = unsafe_set_int32_be
let set_int64_be = unsafe_set_int64_be
let set_int16_be = unsafe_set_int16_be
let set_int32_be = unsafe_set_int32_be
let set_int64_be = unsafe_set_int64_be
end in
let module Blit : S.Blit = struct
open Bigstringaf
Expand Down

0 comments on commit bf75228

Please sign in to comment.