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 dleblancat_private
This archive was generated by hypermail 2b30 : Fri Apr 13 2001 - 13:53:06 PDT