Re: New Microsoft Security Tool for developers

["Dana Epp" <dana () vulscan com>]

Well, I guess how you look at it. I am not fretting if the strncpy is safe,
I am fretting about if the data in str is safe or not.

I would NOT want to exit directly in that function for a few reasons:

1) There would be no chance for the calling function to clean up anything it
was doing.

2) There would be no way to follow a clean failure path from the calling
function to fail securely.

3) The verification should be done closest to the entry and use of data, yet
has to been in scope of the call. You are to deep into the calling stack and
have no way to unwind it and retry it if you exit here.

You are right that the situation would be quite serious if this function
fails. But I don't think it should be the place of the function to fail
securely. I think its the responsibility of the calling function, which can
better determine how to proceed and either retry with different defaults, or
fail securely. In my case if I was to use a function like that, on failure I
would want to record the data in a way to further analyze why it failed all
my previous bounds checks before entering in there. Of course, thats just
me.

---
Regards,
Dana M. Epp
[Blog: http://silverstr.ufies.org/blog/]


----- Original Message ----- 
From: "Jack D. Unrue" <[EMAIL PROTECTED]>
Cc: <[EMAIL PROTECTED]>
Sent: Friday, December 12, 2003 2:52 PM
Subject: Re: [SC-L] New Microsoft Security Tool for developers


> I agree with the NULL pointer check.
>
> Unless you are paranoid that strncpy() is buggy or hijacked, I
> don't believe your second goal is achieved as stated.  But more
> importantly, if in fact there is overflow, then the situation is
> a whole lot more serious than merits simply returning a flag.  I
> would argue that the application should exit (gracefully if possible)
> at that point.  At the same time, if strncpy() is not trustable,
> then you can't trust the C library and that is a grave design
> problem to be solved.
>
> -- 
> Jack Unrue
>
>
> Dana Epp wrote:
> > Liudvikas,
> >
> > On the surface the code doesn't look too bad, and normally we get a
false
> > sense of security when we use the "safer" string functions.
> >
> > The reality is that what is wrong in that code (well, in my opinion
> > atleast), is that you can't really tell if there was an overflow
happening
> > or not, and you can pass in a null which could do weird things. There
are a
> > couple of conditions that should be looked for:
> >
> > 1) What happens if str is null?
> >
> > 2) What happens if str is larger than buff? How do we know we have a
> > problem?
> >
> > A better way to write it would be something like:
> >
> > bool noOverflow(char *str)
> > {
> >      char buffer[10];
> >
> >      if( str == NULL )
> >      {
> >          /* We should never have a NULL string */
> >           assert( false );
> >           return false;
> >      }
> >
> >      /* Lets prep our buffer to check for an overflow. Lets nullify the
end
> > char first */
> >      buffer[sizeof(buffer)-1] = '\0';
> >
> >     /* Lets copy the string in, max of buffer size */
> >     strncpy( buffer, str, sizeof(buffer) );
> >
> >     /* Now lets check if the null at the end of the buffer has been
trampled
> > */
> >     if( buffer[sizeof(buffer)-1] != '\0' )
> >     {
> >           /* We know of an unsafe string. This has overflowed! */
> >           return false;
> >      }
> >
> >      /* Avoiding buffer flow with the above two lines */
> >      return true;
> > }