Code Auditing Questions

[M Bealby]

Hey all,

As my exams are now over I will hopefully be having more free time
available to put into ReactOS security.  I would like to perform a
security audit of the code base.  I've jotted down a few thoughts and
suggestions of mine below and would appreciate feedback on them.


Methodology

I hope to work on the kernel mode code first, working up through the
layers up to application API level code.  I feel this is the best
method because, as it has been mentioned before to me on IRC / email ,
some functions canot check the parameters that are passed down to
lower functions.

I understand the following code to run in kernel mode:

/lib/rtl/
/lib/string/
/lib/kjs/
/lib/freetype/
/lib/pseh/
/hal/
/drivers/
/subsys/win32k/
/ntoskrnl/

If you know of any more please let me know.

I was thinking of checking out an entire svn revision and updating
only after I had checked an entire library.  What do other people
think on this?

I will predominatly be working just from the code source, but after
playing around with Doxygen I have found it to be incredibly useful
for quick referencing typedefs and function parameter calls.


What am I looking for?

I am only a single person, so code auditing will be a slow process at
the moment.  I wish to check for the following vulnerabilities:

Incorrect null termination.
Buffer overflows.
Premature termination.
Lack of input validation.
Bad calculations.
Off by one / few.

Personally I feel this list of six common vulnerability types should
be enough to thwart the most common of attacks, but if anyone else
feels some other class of vulnerability is missing from the list
please let me know.  Obviously if I find any other bugs then they will
be reported as well.


Reporting of errors?

How should I go about reporting the errors?  I was previously told to
report all occasions of the incorrect uses of the RtlAllocateHeap
function under the same bug number (no 1110, some still unfixed).  Is
this the preferred method for my code auditing results?  I would like
to submit the bugs as quickly as possible, so I can keep my working
tree as close to possible to head, but this may mean submitting
multiple bug reports for the same problem.  I think that the best
option is to submit similar bugs in the same module under the same
report, but open a new bug if the same vulnerability occurs in another
module. Thoughts?


These are just some of my ideas that are floating around in my head at
the moment.  I would welcome all feedback on this matter and I think I
may create a wiki page with the same information on.  Again, thoughts
on this would be nice as I haven't really used wiki's before.

Cheers,
Martin

[Casper Hornstrup]

>   What am I looking for?
>  
>  I am only a single person, so code auditing will be a slow process at
>  the moment.  I wish to check for the following vulnerabilities:
>  
>  Incorrect null termination.
>  Buffer overflows.
>  Premature termination.
>  Lack of input validation.
>  Bad calculations.
>  Off by one / few. 
Abuse of KeBugCheck(Ex). I suspect one can still find places where
KeBugCheck(Ex) is incorrectly called. E.g. when a parameter is found to be
invalid.

Casper

[Brandon Turner]

M Bealby wrote:

>  Hey all,
>
> <snip>
>
>
> Reporting of errors?
>
> How should I go about reporting the errors?  I was previously told to
> report all occasions of the incorrect uses of the RtlAllocateHeap
> function under the same bug number (no 1110, some still unfixed).  Is
> this the preferred method for my code auditing results?  I would like
> to submit the bugs as quickly as possible, so I can keep my working
> tree as close to possible to head, but this may mean submitting
> multiple bug reports for the same problem.  I think that the best
> option is to submit similar bugs in the same module under the same
> report, but open a new bug if the same vulnerability occurs in another
> module. Thoughts?
>   
>  
We ask that they go in bugzilla just because just for bookeeping 
purposes and so that it doesnt get lost and forgotten.  Though at time 
bugzilla is neglected.  I will try to keep up with you on the reports 
that you send it. Im sorry I didnt do a better job in the past, I forgot 
about that bug.  Also, if you are in irc and something is small fix that 
you dont want to report to bugzilla, im usually in there and mostly I 
have time here and there to fix things throughout the day.

>  
> These are just some of my ideas that are floating around in my head at
> the moment.  I would welcome all feedback on this matter and I think I
> may create a wiki page with the same information on.  Again, thoughts
> on this would be nice as I haven't really used wiki's before.
>
> Cheers,
> Martin
> _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
>
>
>   
>  
Brandon

[M Bealby]

>  We ask that they go in bugzilla just because just for
> bookeeping
> purposes and so that it doesnt get lost and forgotten.  Though at time
> bugzilla is neglected.  I will try to keep up with you on the reports
> that you send it. Im sorry I didnt do a better job in the past, I forgot
> about that bug.  Also, if you are in irc and something is small fix that
> you dont want to report to bugzilla, im usually in there and mostly I
> have time here and there to fix things throughout the day. 
Hey Brandon,

I wasn't targetting anyone in particular with that post, so I hope I
haven't offended you or anyone else.  I just noticed that the bug was
still open when I checked how I reported them previously.  I know the
developers are always stretched in that they want to always implement
new code to improve the functionality of ReactOS.  I don't think there
will be a flood of bugs filed as it will be a slow process, so don't
worry about that. :)

Everything of a significant size will go through bugzilla.  I just
wasn't sure about how I should report them to it.  Some other projects
like every single instance of every bug to be filed under a different
bug number.  I can see why they like this method but I think it can be
unhelpful.  If no one suggests otherwise I'll use my original
suggested method.

Thanks for the replies so far, they've been very helpful as I'm not
*really* familiar with the code yet.

Martin

[Royce Mitchell III]

M Bealby wrote:

>  I was thinking of checking out an entire svn revision
> and updating
> only after I had checked an entire library.  What do other people
> think on this?
>   
>  
I think documenting code that doesn't check parameters because those
checks are done elsewhere will immensely help future auditors.