apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: [PATCH] avoid crashing when given invalid user/group ids on win32
Date Mon, 30 Jan 2006 17:36:14 GMT
Joe Orton wrote:
> On Sat, Jan 21, 2006 at 05:07:21PM -0800, Garrett Rooney wrote:
> 
>>I'm not sure this is entirely correct, but here's a quick patch to
>>correct the problem I reported earlier about crashing in testuser.c
>>when we pass bogus uid/gid values into apr_uid_name_get and
>>apr_gid_name_get.
>>
>>The fix is to use IsValidSid to confirm the validity of the uid/gid
>>before we try to call LookupAccountSid.
>>
>>The one thing I'm really not sure of is what should be done on non-NT
>>systems.  The MSDN docs say that IsValidSid didn't show up until NT
>>workstation 3.1.  Then again, they say the same thing about
>>LookupAccountSid, and we seem to use that unconditionally, so perhaps
>>it's ok.
> 
> 
> Making the apr_{uid,gid}_name_get() test for an arbitrary apr_uid_t 
> value ifndef WIN32 seems reasonable if it's not possible to obtain such 
> values legitimately on Win32.
> 
> It is possible to obtain such values legitimately on Unix (e.g. random 
> file owner UIDs which show up on an NFS mount); so the test is valid 
> there.

My point was, no, it's not possible; where did you get said random UID?
Perhaps you called apr_file_stat() and it returned a uid to some non
existant user?

That's not the same thing as assuming you know that uid is an int.  If you
take one apr-uid and pass it to these functions, they should not crash.  But
if you take one non-apr uid and pass it in, on unix that works, on Win32
it crashes.

On win32 we've 'hidden' the uid as a pointer to uid stored on the pool it
was queried from.  Nothing wrong with that.  The test is simply data-type
abuse.



Mime
View raw message