Usage of RtlAllocateHeap

[M Bealby]

Hi,

First of all I would like to introduce myself. My name is Martin and I am
currently attempting to go through and audit the ReactOS source code for
security vulnerabilities. I've been keeping an eye on the project for
about a year now and I think as development is now rapidly increasing,
security must take more and more of a priority.

I started with the kernel32 library on Friday (9th) and have found a
number of possible security vulnerabilies. These have been filed with
bugzilla and i'm very impressed with the rate that these were fixed. So
well done on that count!

However, after submitting bugs #1102 and #1103 (now fixed by
BrandonTurner) it became apparent that incorrect usage of the
RtlAllocateHeap function (and its alias HeapAlloc) is quite common. It
may return NULL if memory cannot be allocated and in some parts of the
code this is not checked. This would lead to a segmentation fault on
reading / writing.

Through some judicious use of grep I went through the source code and
extracted calls to this function with their context. After going through
each one and investigating it I have now found another twenty occurances
of the same bug (This is just in the kernel32 library).

I am writing this to let all developers know that they must check their
return values. Errors can exist, resources may be starved and the call
may fail. Make sure your code can cope with this.

Finally, as I am fairly new to bugzilla, should I submit each of these
twenty bugs individually or should I submit them all as one bug?

Cheers,
Martin

[Brandon Turner]

I'm fine with submiting them all as one bug as long as they are in the 
same lib.

Brandon

M Bealby wrote:

>  Hi,
>
> First of all I would like to introduce myself. My name is Martin and I am
> currently attempting to go through and audit the ReactOS source code for
> security vulnerabilities. I've been keeping an eye on the project for
> about a year now and I think as development is now rapidly increasing,
> security must take more and more of a priority.
>
> I started with the kernel32 library on Friday (9th) and have found a
> number of possible security vulnerabilies. These have been filed with
> bugzilla and i'm very impressed with the rate that these were fixed. So
> well done on that count!
>
> However, after submitting bugs #1102 and #1103 (now fixed by
> BrandonTurner) it became apparent that incorrect usage of the
> RtlAllocateHeap function (and its alias HeapAlloc) is quite common. It
> may return NULL if memory cannot be allocated and in some parts of the
> code this is not checked. This would lead to a segmentation fault on
> reading / writing.
>
> Through some judicious use of grep I went through the source code and
> extracted calls to this function with their context. After going through
> each one and investigating it I have now found another twenty occurances
> of the same bug (This is just in the kernel32 library).
>
> I am writing this to let all developers know that they must check their
> return values. Errors can exist, resources may be starved and the call
> may fail. Make sure your code can cope with this.
>
> Finally, as I am fairly new to bugzilla, should I submit each of these
> twenty bugs individually or should I submit them all as one bug?
>
> Cheers,
> Martin
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
>
>
>   
>

[M Bealby]

>  I'm fine with submiting them all as one bug as long
> as they are in the
> same lib. 
They have all been submitted under bug #1110

http://www.reactos.org/bugzilla/show_bug.cgi?id=1110

Cheers,
Martin

[Magnus Olsen]

Hi
ReactOS got plent of bugs over the whole place. 
and some bugs are releate to rtl* functions for they got some bugs. 

----- Original Message ----- 
From: "M Bealby" <mbealby(a)gmail.com>
To: <ros-dev(a)reactos.org>
Sent: den 11 December 2005 15:20
Subject: [ros-dev] Usage of RtlAllocateHeap


>   Hi,
>  
>  First of all I would like to introduce myself. My name is Martin and I am
>  currently attempting to go through and audit the ReactOS source code for
>  security vulnerabilities. I've been keeping an eye on the project for
>  about a year now and I think as development is now rapidly increasing,
>  security must take more and more of a priority.
>  
>  I started with the kernel32 library on Friday (9th) and have found a
>  number of possible security vulnerabilies. These have been filed with
>  bugzilla and i'm very impressed with the rate that these were fixed. So
>  well done on that count!
>  
>  However, after submitting bugs #1102 and #1103 (now fixed by
>  BrandonTurner) it became apparent that incorrect usage of the
>  RtlAllocateHeap function (and its alias HeapAlloc) is quite common. It
>  may return NULL if memory cannot be allocated and in some parts of the
>  code this is not checked. This would lead to a segmentation fault on
>  reading / writing.
>  
>  Through some judicious use of grep I went through the source code and
>  extracted calls to this function with their context. After going through
>  each one and investigating it I have now found another twenty occurances
>  of the same bug (This is just in the kernel32 library).
>  
>  I am writing this to let all developers know that they must check their
>  return values. Errors can exist, resources may be starved and the call
>  may fail. Make sure your code can cope with this.
>  
>  Finally, as I am fairly new to bugzilla, should I submit each of these
>  twenty bugs individually or should I submit them all as one bug?
>  
>  Cheers,
>  Martin
>  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev

[Thomas Weidenmueller]

M Bealby wrote:
>   However, after submitting bugs #1102 and #1103 (now
> fixed by
>  BrandonTurner) it became apparent that incorrect usage of the
>  RtlAllocateHeap function (and its alias HeapAlloc) is quite common. It
>  may return NULL if memory cannot be allocated and in some parts of the
>  code this is not checked. This would lead to a segmentation fault on
>  reading / writing. 
If it was only the incorrect usage of RtlAllocateHeap, missing checks
are all over the place and I wish at least all of our permanent
developers would take care of at least the most common ones that could
lead to crashes in case of failure. Admittedly sometimes people seem to
be lazy and don't care about possible failures, but it's annoying to see
such code going in on an almost daily basis.

- Thomas