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

[Jeremy Allison]

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.

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.

Jeremy.