svn commit: samba r23668 - in branches: SAMBA_3_0/source/lib SAMBA_3_0_26/source/lib

[Michael Adam]

On Sun, Jul 01, 2007 at 08:14:30AM +0200, Volker Lendecke wrote:
> On Sat, Jun 30, 2007 at 05:39:49PM -0700, Jeremy Allison wrote:
> > On Sun, Jul 01, 2007 at 02:04:36AM +0200, Michael Adam wrote:
> > > 
> > > I did not create a security hole (kept at the worst): 
> > > This block of code was just indented one additional level. 
> > > num_ucs2 = length/2, length being passed to the function. 
> > > So there is no danger of wrap here. - right?
> > 
> > Where did length come from ? Please check length.
> 
> length is client-determined here, so definitely tainted (in
> the perl sense). But still I don't see how this can wrap.
> length and num_ucs2 are both unsigned, and right before the
> malloc(num_ucs2+1) num_ucs2 was calculated as
> num_ucs2=length/2. 

This was exactly my reasoning, why num_ucs2 can't wrap.

But of course length itself is potentially dangerous here,
since "length + 2" is passed to convert_string_talloc a
couple of lines further down (since the function adds a
(two byte) zero to the end of the input string.

So what to do about that? Check for length >= 
(maximum of uint32) - 2  and return an error when
this is the case?

Michael

-- 
Michael Adam <ma at sernet.de>
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.SerNet.DE, mailto: Info @ SerNet.DE