[PATCH v3 0/6] samba: fix conversion of strings to binary SIDs and vice versa
jlayton at samba.org
Wed Jul 31 08:38:17 MDT 2013
This is the third posting of this set. Summary of changes:
- add torture tests
- fix bad mask in dom_sid_parse_endp
- eliminate extra '-' in sid-to-string conversions
- fix conversion logic in sid-to-string routines
- fix the libcli versions of these routines
- eliminate unneeded "code cleanup" deltas and casts
- ensure that we add ULL specifiers on long constants
- use UINT32_MAX instead of the more ambiguous UINT_MAX
The original patch description follows:
While implementing a similar routine for cifs-utils, I used the logic in
wbcStringToSid. Unfortunately, I couldn't use it directly due to a
number of problems in this code (some worse than others):
- We're not converting signed integers here, so it makes more sense to
to use strtou* variants.
- The code doesn't check whether the revision string is larger than a
uint8_t can hold. It should.
- It's possible to have legitimate authority bytes in fields 0 and 1,
so we need to convert that value using strtoull and mask and shift
those bytes too.
- We should also ensure that the value in the authority field does not
exceed what the id_auth array can hold. We should also check for it
to return ULONG_MAX. Checking the returned value vs. a newly declared
AUTHORITY_MASK should cover both cases.
- If the authority is >= UINT_MAX then it should be represented as a
hex value (according to MS-DTYP).
- rather than just ignoring high-order bits when converting the
subauthority values, we should instead ensure that it's not
larger than a 32-bit value, and throw an error if it is.
- the check for a NULL endptr value in the subauthority loop seems
to be entirely bogus. It's not clear to me when strtoul would ever
set that to NULL.
Now, all that said...I'm not very familiar with this code and I don't
have a great way to test it so I'd prefer someone with more knowledge
of this code take these patches and shepherd them in.
Jeff Layton (6):
wbclient: fix conversion logic in wbcStringToSid
wbclient: fix conversion logic in wbcSidToStringBuf
libcli: fix conversion logic in dom_sid_parse_endp
libcli: fix conversion logic in dom_sid_string_buf
torture: add more string_to_sid torture testcases
torture: add LOCAL-sid_to_string testcase
libcli/security/dom_sid.c | 57 ++++++++++++++++++++---------------
nsswitch/libwbclient/wbc_sid.c | 68 +++++++++++++++++++++++-------------------
source3/selftest/tests.py | 1 +
source3/torture/torture.c | 46 ++++++++++++++++++++++++++++
4 files changed, 117 insertions(+), 55 deletions(-)
More information about the samba-technical