As a die-hard native programmer I was always disgusted by integer overflow checks and array bounds checks. Littering code with branches everywhere? Just let me go as fast possible please!

Last week I was explained by security people how a part of vulnerabilities todays are attacks on image parsers, and how integer overflow checks may help there.

IIRC a typical attack on image format parser is to forge an image with a width and height that will overflow an int.

On allocation, the result of the multiplied wraps around like this:

    int width = parse_width_from_stream();     // eg: 131072
    int height = parse_height_from_stream();   // eg: 131073
    ubyte[] data = malloc(width * height * 4); // wraps around, allocates way less memory than that

This prepare the program to access data that is outside the truly allocated area, for example in another block.
The other part of the job is to actually jump in the injected code, but I've not understood this part (somehow implied knowing how the specific allocator work in this case). Somehow integer overflows are part of sophisticated attacks for which it's a building block.

If overflow checks happen to be more or less cheap like (surprinsingly) array bounds checks are, it could be a nice thing to pay for.

References:
- Integer Overflow into Information Disclosure http://nullprogram.com/blog/2017/07/19/
- Basic Integer Overflows http://phrack.org/issues/60/10.html

On Friday, 15 September 2017 at 08:46:57 UTC, Guillaume Piolat wrote:
>     int width = parse_width_from_stream();     // eg: 131072
>     int height = parse_height_from_stream();   // eg: 131073

Do you hope to see such code? Since width can't be negative, C programmer would use unsigned integer for it, and you can't prohibit overflow for unsigned integer. It is unfixable for array length, because unsigned integers are invariably used for length. Blueborn vulnerabilities rely on overflow of unsigned integers (for buffer length) to trigger buffer overflow in calls to memcopy. But buffer overflow would normally be prevented by bound checks in case of integer overflow. Just have a safer wrapper around malloc in your example.

On Friday, 15 September 2017 at 12:04:27 UTC, Kagamin wrote:
> Do you hope to see such code? Since width can't be negative, C programmer would use unsigned integer for it, and you can't prohibit overflow for unsigned integer. It is unfixable for array length, because unsigned integers are invariably used for length. Blueborn vulnerabilities rely on overflow of unsigned integers (for buffer length) to trigger buffer overflow in calls to memcopy.

This code isn't to be taken literally, the important bit is that silent integer overflow allows this kind of attacks.


> But buffer overflow would normally be prevented by bound checks in case of integer overflow.

Well here I don't think so: this attack is used to adress a very large space, while having a very small actually allocated memory space. Bounds would be too large to matter.

> have a safer wrapper around malloc in your example.

That would be calloc.
The point is that it's easy to make the vulnerability disappear, once you know about such things and traps. It falls under the "unknown unknowns" category of risk most of the time though.

On Friday, 15 September 2017 at 12:04:27 UTC, Kagamin wrote:
> Since width can't be negative, C programmer would use unsigned integer for it

That's often a big mistake. Lots of people do it... but you shouldn't, exactly because of the wraparound behavior.

On Friday, 15 September 2017 at 12:33:56 UTC, Adam D. Ruppe wrote:
> On Friday, 15 September 2017 at 12:04:27 UTC, Kagamin wrote:
>> Since width can't be negative, C programmer would use unsigned integer for it
>
> That's often a big mistake. Lots of people do it... but you shouldn't, exactly because of the wraparound behavior.

Signed integers have the same issue, no? Using int32s:

2147483635 * 400000000 * 4 = 674836480

It's just that you're more likely to be able to detect it, since a lot of inputs result in negative numbers.

One solution is to switch to the next larger integer size. That works up to 32-bit numbers. When you hit 64-bit, you've got to switch to BigInt.

Another solution is to check the overflow bit as appropriate. The checkedint package automates that.

The last solution that I can think of, specific to this type of thing, is to use the result to allocate a bounds-checked array, where the allocation function yields the appropriately sized array. You'll get an array bounds error that may be inscrutable.

On Friday, 15 September 2017 at 12:25:10 UTC, Guillaume Piolat wrote:
> Well here I don't think so: this attack is used to adress a very large space, while having a very small actually allocated memory space. Bounds would be too large to matter.

As long as it works in bounds it should be more or less ok.

> That would be calloc.

I mean allocator that returns bound checked array. And you can call calloc incorrectly too.

> The point is that it's easy to make the vulnerability disappear, once you know about such things and traps.

It's not because nobody knows about buffer overflows. C leaves the task on the programmer, and the task is too huge for manual labor without help from the language, ecosystem and coding practices, of course nobody does it.

On Friday, 15 September 2017 at 16:04:08 UTC, Neia Neutuladh wrote:
> The last solution that I can think of, specific to this type of thing, is to use the result to allocate a bounds-checked array, where the allocation function yields the appropriately sized array. You'll get an array bounds error that may be inscrutable.

It's true that my example doesn't fail in the same way if eg.

   new ubyte[width * height]

was used.

Point taken about proper allocator with returned size. It's indeed a minority of cases in regular D.

On Friday, 15 September 2017 at 16:39:46 UTC, Kagamin wrote:
>
> It's not because nobody knows about buffer overflows. C leaves the task on the programmer, and the task is too huge for manual labor without help from the language, ecosystem and coding practices, of course nobody does it.

I was wondering because some image libraries do use malloc(), for the decreased memory usage and possibility of false positives.

On 9/15/2017 1:46 AM, Guillaume Piolat wrote:
> If overflow checks happen to be more or less cheap like (surprinsingly) array bounds checks are, it could be a nice thing to pay for.

https://dlang.org/phobos/std_experimental_checkedint.html

On Friday, 15 September 2017 at 21:21:01 UTC, Walter Bright wrote:
> On 9/15/2017 1:46 AM, Guillaume Piolat wrote:
>> If overflow checks happen to be more or less cheap like (surprinsingly) array bounds checks are, it could be a nice thing to pay for.
>
> https://dlang.org/phobos/std_experimental_checkedint.html

Will this ever be integrated directly into the compiler?