While D offers buffer overflow detection, it does not protect against buffer overflows resulting from an array size calculation overflow:

    T* p = cast(T*)malloc(len * T.sizeof);

What if `len*T.sizeof` overflows? malloc() will succeed, but the result will be too small for the data.

I decided to grep dmd for such allocations:

https://github.com/dlang/dmd/pull/13479/files

and fix them with overflow checks. I recommend everyone check their own projects and eliminate such vulnerabilities.

I post this as I've recently seen reports on malware injection being enabled by presenting specially crafted input data to a program that causes an overflow on the allocation, then overwrites the data beyond the truncated allocated memory.

On Friday, 31 December 2021 at 00:13:56 UTC, Walter Bright wrote:
> While D offers buffer overflow detection, it does not protect against buffer overflows resulting from an array size calculation overflow:
>
>     T* p = cast(T*)malloc(len * T.sizeof);

What I do in D is always slice the malloc to the given size immediately;

T[] p = (cast(T*)malloc(len * T.sizeof))[0 .. len * T.sizepf];

Then you'd get the protection of bounds checking and if you need the ptr, there's still that property.

I'd suggest everyone always do that.

On Friday, 31 December 2021 at 00:15:48 UTC, Adam Ruppe wrote:
> T[] p = (cast(T*)malloc(len * T.sizeof))[0 .. len * T.sizepf];

eeeek, I did it wrong!

Should be either [0 .. len] on the slice or do the cast on the outside instead of inside of parens.

This is why just using `new` is the best of all!

On Friday, 31 December 2021 at 00:15:48 UTC, Adam Ruppe wrote:
> On Friday, 31 December 2021 at 00:13:56 UTC, Walter Bright wrote:
>> While D offers buffer overflow detection, it does not protect against buffer overflows resulting from an array size calculation overflow:
>>
>>     T* p = cast(T*)malloc(len * T.sizeof);
>
> What I do in D is always slice the malloc to the given size immediately;
>
> T[] p = (cast(T*)malloc(len * T.sizeof))[0 .. len * T.sizepf];
>
> Then you'd get the protection of bounds checking and if you need the ptr, there's still that property.
>
> I'd suggest everyone always do that.

Good thing to do, but Walter's talking about integer overflow with the `len * T.sizeof` calculation itself.

calloc() doesn't have this problem.

On Fri, Dec 31, 2021 at 12:34:46AM +0000, Adam Ruppe via Digitalmars-d wrote:
> On Friday, 31 December 2021 at 00:15:48 UTC, Adam Ruppe wrote:
> > T[] p = (cast(T*)malloc(len * T.sizeof))[0 .. len * T.sizepf];
> 
> eeeek, I did it wrong!
> 
> Should be either [0 .. len] on the slice or do the cast on the outside instead of inside of parens.
[...]

Actually, if (len * T.sizeof) overflows, then neither [0 .. len] nor
[0 .. len * T.sizeof)] would be safe from buffer overruns.

E.g., if len = size_t.max / 4 and T.sizeof = 8, then (len * T.sizeof) would wrap around to a much smaller value than expected, which is the problem Walter is trying to point out.


T

-- 
That's not a bug; that's a feature!

On Friday, 31 December 2021 at 00:37:20 UTC, sarn wrote:
> Walter's talking about integer overflow with the `len * T.sizeof` calculation itself.

Slicing should have its own overflow check, meaning it doesn't matter if the other calculation overflows.  (I don't know if it does, but should.)

On 31/12/2021 1:37 PM, sarn wrote:
> On Friday, 31 December 2021 at 00:15:48 UTC, Adam Ruppe wrote:
>> On Friday, 31 December 2021 at 00:13:56 UTC, Walter Bright wrote:
>>> While D offers buffer overflow detection, it does not protect against buffer overflows resulting from an array size calculation overflow:
>>>
>>>     T* p = cast(T*)malloc(len * T.sizeof);
>>
>> What I do in D is always slice the malloc to the given size immediately;
>>
>> T[] p = (cast(T*)malloc(len * T.sizeof))[0 .. len * T.sizepf];
>>
>> Then you'd get the protection of bounds checking and if you need the ptr, there's still that property.
>>
>> I'd suggest everyone always do that.
> 
> Good thing to do, but Walter's talking about integer overflow with the `len * T.sizeof` calculation itself.
> 
> calloc() doesn't have this problem.

I would argue any usage of malloc and even calloc is itself a bug in your code.

Ideally we would deprecate malloc and calloc from being called directly and force people to use a callocSlice version of it instead.

This solves most of these issues out right and allows for D's memory safety features to come into play.

On Friday, 31 December 2021 at 01:02:36 UTC, H. S. Teoh wrote:
> [0 .. len * T.sizeof)] would be safe from buffer overruns.

If you do this correctly on the byte before casting you SHOULD be ok because if the malloc overflows, so will the slice calculation, meaning you get the RangeError when you try to use it, which is what we want.

But since I screwed it up twice already i say this is evidence to just use `new` and find joy :P

On Friday, 31 December 2021 at 00:13:56 UTC, Walter Bright wrote:
> While D offers buffer overflow detection, it does not protect against buffer overflows resulting from an array size calculation overflow:
>
>     T* p = cast(T*)malloc(len * T.sizeof);
>
> What if `len*T.sizeof` overflows? malloc() will succeed, but the result will be too small for the data.
>
> I decided to grep dmd for such allocations:
>
> https://github.com/dlang/dmd/pull/13479/files
>
> and fix them with overflow checks. I recommend everyone check their own projects and eliminate such vulnerabilities.
>
> I post this as I've recently seen reports on malware injection being enabled by presenting specially crafted input data to a program that causes an overflow on the allocation, then overwrites the data beyond the truncated allocated memory.

are the asserts not taken out in release mode?

On Friday, 31 December 2021 at 08:46:59 UTC, claptrap wrote:
> On Friday, 31 December 2021 at 00:13:56 UTC, Walter Bright wrote:
>> [...]
>
> are the asserts not taken out in release mode?

The compiler ships with assertions turned on.