Fix string_to_sid() to allow non '\0' termination of the string

[Andrew Bartlett]

On Tue, 2010-09-14 at 15:43 -0700, Jeremy Allison wrote:
> On Wed, Sep 15, 2010 at 08:38:54AM +1000, Andrew Bartlett wrote:
> > On Tue, 2010-09-14 at 23:51 +0200, Jeremy Allison wrote:
> > > The branch, v3-6-test has been updated
> > >        via  c4a31cf Fix string_to_sid() to allow non '
> > >        via  ea8f73f s3-torture Add tests to show that the dom_sid parsing was faulty.
> > >        via  b45b538 s3-util_sid Use the NDR parser to parse struct dom_sid
> > >        via  dad0b14 libcli/security Use sid_append_rid() in dom_sid_append_rid()
> > >        via  4ac32a5 libcli/security Merge source3/ string_to_sid() to common code
> > >        via  9e31c9a s3-util_sid use ARRAY_SIZE() to ensure we never overflow the dom_sid
> > >        via  1ac4f6a s3-util_sid Accept S-1-5 as a SID (cherry picked from commit 9d44688681bc196baf1bccbdf84092ffc0510bb7)
> > >        via  0dc0a81 s3-dom_sid Use C99 types in dom_sid handling
> > >       from  f4c8bda Fix bug 7409 - Thousands of reduce_name: couldn't get realpath.
> > > 
> > > http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-6-test
> > > 
> > > 
> > > - Log -----------------------------------------------------------------
> > > commit c4a31cf4d6b1a7c342ed223bdbab3dbd21073f5d
> > > Author: Jeremy Allison <jra at samba.org>
> > > Date:   Tue Sep 14 14:45:45 2010 -0700
> > > 
> > >     Fix string_to_sid() to allow non '\0' termination of the string - allows
> > >     string_to_sid() to be used in formatted strings like FOO/S-1-5-XXXX-YYYY/BAR.
> > >     
> > >     Jeremy.
> > >     (cherry picked from commit 55b315094ef8a8ed691f9717c28cab301e17ef25)
> > 
> > Firstly, thanks for merging these changes.  
> > 
> > However, while I can't argue with 'existing code needs this', I wonder
> > if this is really desirable behaviour?  Shouldn't we have the caller
> > trim off the string, or call a function with a length specified?  It
> > just seems we may loose an important guard against invalid input this
> > way.
> > 
> > What do you think?
> 
> Personally I think it makes the function easier to use.
> In order to trim the string at the right place you'd have to parse it
> anyway, which pre-supposes knowledge of a SID string in the
> caller.

Good point. 

> IMHO The correct change to this function is to add a
> char **end pointer as a return, so that it works in the
> same way as strtoul(), and callers can know what part of
> the passed in string has been consumed as the SID.
> 
> There's no guard against invalid input by insisiting
> on a '\0' terminator, it's just a burdon on the caller.

Yeah.  I'll see about such a change, and having a length-limited and
asserting caller or variant for some of the uses like ldb where I would
like a tighter control on this. 

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20100915/12c7b7b8/attachment.pgp>