Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect integer overflow in Array.concat #810

Merged
merged 2 commits into from
Sep 26, 2016

Conversation

yallop
Copy link
Member

@yallop yallop commented Sep 13, 2016

This pull request is part of an occasional series on bounds checking and integer overflow. (Previous installments: #250, #805.)

Description

The code used to calculate the size of the output array in Array.concat doesn't detect integer overflow. Consequently, passing a single (moderately-sized) list with multiple references to a large array can lead to out-of-bounds writes.

Demonstration

Here's a program that demonstrates the problem on a 32-bit platform.

open Int32

let big = Array.make Sys.max_array_length ()

let push x l = l := x :: !l
let (+=) a b = a := add !a b

let sz, l = ref zero, ref []

let () = begin
  while !sz >= zero do push big l; sz += of_int Sys.max_array_length done;
  while !sz <= zero do push big l; sz += of_int Sys.max_array_length done;
end

let _ = Array.concat !l

And here's the result of compiling and running the program:

$ ocamlopt ~/arrayconcat.ml
$ ./a.out
Segmentation fault (core dumped)

Here's what happens with the addition of this patch:

$ ocamlopt ~/arrayconcat.ml
$ ./a.out 
Fatal error: exception Invalid_argument("Array.concat")

@damiendoligez damiendoligez merged commit 89aff47 into ocaml:trunk Sep 26, 2016
@yallop yallop deleted the array-bounds branch September 26, 2016 13:09
damiendoligez pushed a commit that referenced this pull request Sep 26, 2016
* Check for overflow in caml_array_gather.

* Add a changelog entry for the check for overflow in Array.concat.
@damiendoligez
Copy link
Member

Reviewed, merged, and cherry-picked to 4.04 (commit 0b76fa9).

camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017
* Check for overflow in caml_array_gather.

* Add a changelog entry for the check for overflow in Array.concat.
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants