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.


Reply via email to