KPDF suspicious code Re: Suspicious code in kdegraphics-3.5.2

[Albert Astals Cid]

A Dissabte 22 Abril 2006 01:36, Christoph Bartoschek va escriure:
> ------------------------------------------------------------------
> Misc problems:
> ------------------------------------------------------------------
>
> - kpdf/part.cpp:642
>
> Very strange: !m_document->currentPage() < 1   is false for the first
> page.

slotPreviousPage is not supposed to do anything when you are on the first 
page, so that's not a problem,

> - kpdf/xpdf/fofi/FoFiType1.cc:168
>
> line controls the for loops in lines 168 and 142

Not a real problem, just an ugly construct.

> - kpdf/xpdf/xpdf/Gfx.cc:1919
>
> If i is 255 and j bigger than 256, then there is an out of bounds write
> here.

That's not possible as j is extracted from next[] array and next[] values are 
either the j from the last iteration or (i + j) / 2 and as i will be as much 
255, j should be 259 to get a next[] with value >= 257. and that can not 
happen as we just "demostrated" it will not even reach 257 :D

> - kpdf/xpdf/xpdf/CharCodeToUnicode.cc:253
>
> If n1 is 0 then the index is out of bounds.

Not going to happen as you have 
n1 == 2 + nDigits
where 
nDigits = nBits / 4;
and nBits is a parameter that is always 8 or 16, so if that condition is not 
true, the other where n1 -1 is used will not be evaluated.

> - kpdf/xpdf/xpdf/Function.cc:1280
>
> If i2 is smaller than 0, then the shift amount is invalid here.

I did not write that code, but seems wrong "on purpose"?

i2 = stack->popInt();
i1 = stack->popInt();
if (i2 > 0) {
  stack->pushInt(i1 << i2);
} else if (i2 < 0) {
  stack->pushInt((int)((Guint)i1 >> i2));        <------------
} else {
  stack->pushInt(i1);
}

Maybe it should be  stack->pushInt((int)((Guint)i1 << -i2)); ?

I'll try to reach the original author.

>
> - kpdf/xpdf/xpdf/Annot.cc:141
>
> i1 controls the loops in line 139 and 141.

Again just strange construct, nothing inherently wrong.

> - kpdf/xpdf/xpdf/GfxState.cc:819
>
> nCompsA can be up to 32 according to lines 789/792. If the value is
> bigger than 3 then here is an out of bounds access.

That probably should never happen but i've added a guard anyway.

> - kpdf/xpdf/xpdf/SplashOutputDev.cc:1079
>
> If line 1017 is false, then line 1018 is not executed and tmpBufLen is
> not initialized here.

Not a problem, 1079 is only executed when filename is false and that only 
happens when we had an embeded font file and so tmpBufLen will be initialized

>
> ------------------------------------
> Problems involving the NULL pointer:
> ------------------------------------
>
> - kpdf/core/generator_pdf/generator_pdf.cpp:221
>
> If pdfdoc is NULL as indicated by line 206, then line 221 crashes.

Yeah, right, i think pdfdoc can never be NULL there but i'll move the code 
inside the if :-)

> - kpdf/core/generator_pdf/generator_pdf.cpp:803
>
> If destination is NULL as indicated by line 793 but g->getNameDest() is
> false, then line 803 crashes.

That can not happen, if a link has no destination, it has namedDestination by 
design, but i've added a guard anyway.

> - kpdf/part.cpp:874
>
> If page is NULL as indicated by line 839, then line 874 crashes.

If page is NULL reallyShow is false so that code never gets executed

> - kpdf/xpdf/xpdf/SplashOutputDev.cc:1136
>
> If fileName is NULL as indicated by line 1076, then line 1136 crashes.

if dfp is not null line 1133 filename is never null either

> -----------------------------------------------------------------
> Cases from switch statements that fall through in some cases but
> do not have a fall through comment as in most such cases.
> ------------------------------------------------------------------
> - kpdf/xpdf/xpdf/JPXStream.cc:1600
Added

> - kpdf/ui/pageview.cpp:724
Added

Thanks for the report :-)

Albert

	
	
		
______________________________________________ 
LLama Gratis a cualquier PC del Mundo. 
Llamadas a fijos y móviles desde 1 céntimo por minuto. 
http://es.voice.yahoo.com