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
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@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
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
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@gmail.com To: ros-dev@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@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
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
-
Brandon Turner -
M Bealby -
Magnus Olsen -
Thomas Weidenmueller