[poppler] poppler/Stream.cc

[Albert Astals Cid]

El dimecres, 23 de maig de 2018, a les 21:46:02 CEST, Adam Reichold va escriure:
> Hello again,
> 
> attached the patch. It declares inputBuf as unsigned so all bit shifts
> happen on unsigned values. ctest at least seems to be happy.

Interestingly the automagic fuzzer service says that the issue with LZWStream::getCode doing left shift on negative values has been fixed by a commit in this range
https://cgit.freedesktop.org/poppler/poppler/diff/?id2=76820f5ab932a9ed18913bc7d1a452ddf060c133&id=f966b9096d046aaee4891de11f74207218cc929b

So i guess for not better not to touch this code.

Thanks for the patch though :)

Cheers,
  Albert

> 
> It does build without the casts as well but I am not completely sure
> about the language legalese behind this and hence left them in and also
> for explicitness.
> 
> Proper fix would probably be to converted all of the LZW decoding to use
> unsigned values.
> 
> Best regards,
> Adam
> 
> Am 23.05.2018 um 21:24 schrieb Albert Astals Cid:
> > El dimecres, 23 de maig de 2018, a les 8:57:27 CEST, Adam Reichold va
> > 
> > escriure:
> >> Hello,
> >> 
> >> maybe the simplest solution would to turn inputBuf into an unsigned int
> >> and convert to signed int after extracting the bits out of it?
> > 
> > Yeah that sounds like a plan, could you try to produce a patch so i can
> > run it through regtest?
> > 
> > Cheers,
> > 
> >   Albert
> >> 
> >> Best regards,
> >> Adam
> >> 
> >> Am 23.05.2018 um 00:24 schrieb Albert Astals Cid:
> >>>  poppler/Stream.cc |    4 +---
> >>>  1 file changed, 1 insertion(+), 3 deletions(-)
> >>> 
> >>> New commits:
> >>> commit 58e056c4b15f262b7715f8061d6885eb80044d0d
> >>> Author: Albert Astals Cid <aacid at kde.org>
> >>> Date:   Wed May 23 00:23:19 2018 +0200
> >>> 
> >>>     Revert 31c3832b996acbf04ea833e304d7d21ac4533a57
> >>>     
> >>>     So shifting left negative values is undefined behaviour according to
> >>>     the
> >>>     spec but if we don't do it we break, so we seem to be depending on
> >>>     this
> >>>     undefined behaviour, will try to figure out a better fix
> >>> 
> >>> diff --git a/poppler/Stream.cc b/poppler/Stream.cc
> >>> index b6bfd838..4f075c12 100644
> >>> --- a/poppler/Stream.cc
> >>> +++ b/poppler/Stream.cc
> >>> @@ -1445,9 +1445,7 @@ int LZWStream::getCode() {
> >>> 
> >>>    while (inputBits < nextBits) {
> >>>    
> >>>      if ((c = str->getChar()) == EOF)
> >>>      
> >>>        return EOF;
> >>> 
> >>> -    if (likely(inputBuf >= 0)) {
> >>> -        inputBuf = (inputBuf << 8) | (c & 0xff);
> >>> -    }
> >>> +    inputBuf = (inputBuf << 8) | (c & 0xff);
> >>> 
> >>>      inputBits += 8;
> >>>    
> >>>    }
> >>>    code = (inputBuf >> (inputBits - nextBits)) & ((1 << nextBits) - 1);
> >>> 
> >>> _______________________________________________
> >>> poppler mailing list
> >>> poppler at lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/poppler