X-RDate: Tue, 12 May 1998 11:58:18 +0600 (YEKST)
X-UIDL: 35317d3400000231
Date: Sun, 10 May 1998 17:54:56 -0400
From: David LeBlanc <[email protected]>
To: [email protected]Subject: Re: Samba problems
At 12:43 PM 5/10/98 -0400, Drago wrote:
>I have seen alot of issues about strcpy() and how strncpy() should be used
>instead. Very few times have I seen anything about sprintf()/snprintf()
>which also has the same issues that people have with strcpy() as far as
>buffer overflows go. An easy fix for this is to simply change line 3206
>to use snprintf(). In many other area's of reply.c are the same problems
>that are in reply_mv (reply_unlink(), and a few others).
Actually, what I prefer in this sort of case is to simply check the inputs
- i.e.:
if(strlen(directory) + strlen(dname) + 2 > /*allow 1 for /, another for null*/
1024 /*or sizeof(whatever)*/)
{
complain loudly;
return error;
}
I have the same sort of beef with strncpy - if you overflow a strncpy, it
won't null terminate, and snprintf will do the same thing. You may no
longer have an exploitable condition, but if your string isn't terminated,
the code is going to crash (might even be exploitable under a worst case).
At least snprintf returns an error if it can't fill fit everything -
strncpy won't. Probably the best case would be to have it look like so:
if(snprintf(blah, blah...) < 0)
{
wail;
complain;
}
Having secure code that crashes isn't any good, and is probably vulnerable
to denial of service. Just overfilling the buffer and letting the next
call down the line think it is getting good data is a Bad Thing - better to
detect that someone has fed you junk, and pass an error back to the caller.
I can't say I recall anyone pointing out this er, "feature" of strncpy,
but plenty of folks touting it as a cure-all for insecure code.
if(strlen(input) < bufsize)
{
strcpy(buf, input);
}
else
{
complain();
return;
}
is much more solid code than:
strncpy(buf, input, bufsize);
>Someone feel free to try this against a windows machine, I haven't had a
>chance to try it. The program I included can be used to test a mounted
>samba fs.
This would be hard to do - probably have to try this from a patched samba
mount of the NT share. We run into a number of issues -
1) Our command line tops out at 1024 characters. Since we don't do inline
expansion like UNIX does, this isn't normally a huge difficulty - but it
does mean I can't test this from a command line.
2) Our rename Win32 API (MoveFileEx()) is properly error-checked against
this sort of nonsense. If you feed it a humongous buffer, it just returns
an error.
3) Win95 won't even tolerate a full path of 256 characters, so...
4) NT will tolerate a path that long, but does have some limit - could be
1024 - IIRC, any given directory name tops out at 256. Hmmm - after a bit
of digging, the underlying API limits the filename to 256 characters unless
you turn off path parsing - what happens after that isn't especially well
documented - (after some more digging) turns out that an individual file or
directory can only have 256 characters - no idea what happens if you have
lots of these. Might be interesting to see what happens if you try to
rename an NT directory to > 256 from a UNIX box. Maybe I'll give that a
try later - or write some sort of evil recursion that keeps creating
subdirectories.
David LeBlanc
[email protected]
