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 <[email protected]> > >>> 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 > >>> [email protected] > >>> https://lists.freedesktop.org/mailman/listinfo/poppler _______________________________________________ poppler mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/poppler
