[PATCH] s3-lib/util: fix read across end of namelist string

Michael Adam obnox at samba.org
Tue Apr 8 17:16:38 MDT 2014


Hi Jeremy,

On 2014-04-08 at 10:42 -0700, Jeremy Allison wrote:
> On Tue, Apr 08, 2014 at 10:26:25AM -0700, Jeremy Allison wrote:
> > On Tue, Apr 08, 2014 at 10:25:40AM +0200, Bjoern Baumbach wrote:
> > > Hi!
> > > 
> > > If the namelist, which set_namearray receives, is not terminated with a
> > >  '/', we try to read the next character behind the string termination '\0'.
> > > In the case that the namelist is followed by a (more or less) valid
> > > string, we could produce several effects like unintentional vetoed,
> > > hidden or non-oplocked files or failures like:
> > > "Conversion error: Incomplete multibyte sequence(..."
> > > 
> > > Please find attached the proposed patch :-)
> > 
> > Reviewed-and-pushed. Congratulations - that
> > was a *GREAT* catch ! Thanks a lot Bjoern.

Indeed! I was part of the debugging and I wonder
how we have survived this so long... :-)

> What do you think of the following additional
> patch ? Your fix addresses the problem and
> fixes the bug, but leaves the faulty logic
> inside of the while loops. This patch fixes
> the faulty logic as well, leaving the code
> (hopefully) cleaner and easier to follow,
> but it isn't strictly needed.
> 
> Comments + reviews ? Is it worth adding ?

This is definitely an improvement.

==> "Reviewed-by: Michael Adam <obnox at samba.org>"

If I get it right, this patch would also
fix the bug w/o Björn's namelist_end patch, correct?
But I think we can keep them both.

I can push it to master, but I have a small addition,
making the if (name_end) check explit to read
if (name_and != NULL)  - attached.

Jeremy, feel free to add it on top if you like it
or squash it with your patch which touches this
context anyways.

Cheers - Michael


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-lib-util-make-NULL-check-explicit-in-set_namearra.patch
Type: text/x-diff
Size: 758 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140409/868db831/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 215 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140409/868db831/attachment.pgp>


More information about the samba-technical mailing list