Bug#893806: tiff: CVE-2018-8905: heap-based buffer overflow in LZWDecodeCompat
Well, upstream just published a fix for this issue. This is not really what I wanted to do, but, my bad, I took too much time to submit my fix. I'm probably going to prepare an LTS upload for this patch, if needed I can also backport the fix to Jessie and other versions. Cheers, Hugo -- Hugo Lefeuvre (hle)|www.owl.eu.com 4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA signature.asc Description: PGP signature
Bug#893806: tiff: CVE-2018-8905: heap-based buffer overflow in LZWDecodeCompat
> So, what is the solution ? > > First, change > > TIFFReadScanline(in, buf, row, s) < 0 > > in tools/tiffcp.c to > > TIFFReadScanline(in, buf, row, s) <= 0 > > It is important to understand that 0 is also an error code here. Otherwise, > change error handling in tif_lzw to return negative numbers. After taking a fresh look at it, this turns out to be a non solution because TIFFReadScanline (the function between LZWDecodeCompat and DECLAREcpFunc) converts 0 return codes to -1s. > Then, there's maybe something to fix in tif_lzw, but I'm not quite sure > at the moment and I still have to check that. So, obviously the solution is in tif_lzw. Because of the ignore option, we must understand the process of exiting not only as returning, but most importantly as preparing the decoder for being called again with the same corrupted memory state and the same buggy codes. If we don't do that the next decoder call will stumble across the corrupted memory and crash. There are two things we have to handle: First, make sure the next decoder call detects that it was previously interrupted and that the oldcodep is invalid, so we avoid the crash. Then, we have to be able to skip the buggy error code, because otherwise we are going to run into an endless loop and we do not want that either. I suggest to fix both issues via an additional flag in the decoder base state. This flag would indicate the position where an invalid code can be found. Whenever this flag is set, we would fast forward to the next CODE_CLEAR after the invalid code (according to my understanding of LZW, the whole set of information between the corrupted code and the next CODE_CLEAR may be corrupted or risky / hard to recover). Concerning the Wheezy version, I have inspected the diffs and it is still pretty unclear to me why the issue isn't reproducible because the affected code it present. I'm still working on it. Cheers, Hugo -- Hugo Lefeuvre (hle)|www.owl.eu.com 4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA signature.asc Description: PGP signature
Bug#893806: tiff: CVE-2018-8905: heap-based buffer overflow in LZWDecodeCompat
Hi, > It looks like this buffer overflow is the consequence of an earlier buffer > overflow in the GetNextCodeCompat macro: Hum, that was not completely true. But I think I got it now. The buggy sequence is: 1) Initialy oldcodep points to 0x63201890. We get code 0x010c = 268, add it to the code table, print some stuff and read the next code. 2) The next code is 0x100, which is CLEAR_CODE, so we set free_entp to 0x63201820 (= sp->dec_codetab + CODE_FIRST). Then, we memset free_entp with a size of CSIZE - CODE_FIRST = (MAXCODE(BITS_MAX)+1024L)-258 = (MAXCODE(9)+1024L)-258 = ((1L<<(9))-1)+1024L-258 = 1277 This means that everything between 0x63201820 and 0x63201D1D will be set to 0. Then we read the next code. 3) Next code is CLEAR_CODE, same as before, nothing changes. 4) Next code is 0x01d6. This is bad because it is too big for a first code. At this point we exit 0 without updating the decoder state. 5) Because we exit 0, TIFFReadScanline(in, buf, row, s) < 0 in tools/tiffcp.c is false and the whole process is repeated ! 6) So, again, oldcodep initially points to 0x63201890. BUT, remember, this area was set to 0 by 2) ! So, well, we read the next code which is 0x010c and add it to the code table. This means that for our 0x010c code @codep, codep->next will point to the memset-ed area ! This means that codep->next->next == NULL. 7) Overflow, NULL pointer dereference, etc. So, what is the solution ? First, change TIFFReadScanline(in, buf, row, s) < 0 in tools/tiffcp.c to TIFFReadScanline(in, buf, row, s) <= 0 It is important to understand that 0 is also an error code here. Otherwise, change error handling in tif_lzw to return negative numbers. Then, there's maybe something to fix in tif_lzw, but I'm not quite sure at the moment and I still have to check that. Now that the issue is clear to me, I'll check the Wheezy code and try to see why the issue isn't reproducible there. Cheers, Hugo -- Hugo Lefeuvre (hle)|www.owl.eu.com 4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA signature.asc Description: PGP signature
Bug#893806: tiff: CVE-2018-8905: heap-based buffer overflow in LZWDecodeCompat
It looks like this buffer overflow is the consequence of an earlier buffer
overflow in the GetNextCodeCompat macro:
> #define GetNextCodeCompat(sp, bp, code) { \
> nextdata |= (unsigned long) *(bp)++ << nextbits;\
> nextbits += 8; \
> if (nextbits < nbits) { \
> nextdata |= (unsigned long) *(bp)++ << nextbits;\
> nextbits += 8; \
> } \
> code = (hcode_t)(nextdata & nbitsmask); \
> nextdata >>= nbits; \
> nextbits -= nbits; \
> }
The raw data buffer is read using the bp pointer without proper bound checking.
At some point, we start to read garbage, store it into the code variable which
is later used to create the codep. This invalid codep later triggers the second
overflow.
So now the question is: Why is this first buffer overflow happening ?
My guess is that the sample is declaring more strips than actually available, or
declares strips with incorrect size. I still have to check that however.
Regards,
Hugo
--
Hugo Lefeuvre (hle)|www.owl.eu.com
4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA
signature.asc
Description: PGP signature
Bug#893806: tiff: CVE-2018-8905: heap-based buffer overflow in LZWDecodeCompat
Hi,
My current understanding of the problem (based on investigations on
latest master, but also valid for older versions):
The code_t string type is defined as a kind of chained list. Each entry
contains:
. a pointer to the next string entry
. a length field indicating the remaining length of the string
This length field includes the current entry.
. a value field containing the current string character
. the first character of the string
In the affected source code
> do {
> *--tp = codep->value;
> } while ( (codep = codep->next) != NULL );
we read such a string and put the values into a buffer (in reverse order, but,
well, it doesn't matter).
Here we assume that the string always has the size it claims to have in its
length field.
But it's not the case with the reproducer. There is a list that claims to have
only one character but defines two, so we continue to read and write to the
buffer even if we are already OOB.
So now the question is: why is that happening ? Is it directly user input or is
coming from an earlier bug in the source code ?
Even if I can't reproduce the issue with the original reproducer, the
versions in Wheezy, Jessie and Stretch are likely to be affected because
the affected code is present.
I'm going to continue my investigations, try to prepare a poc for older
versions, prepare a patch for latest master and backport it to Wheezy
(+ Jessie & Stretch if needed).
Regards,
Hugo
--
Hugo Lefeuvre (hle)|www.owl.eu.com
4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA
signature.asc
Description: PGP signature
Bug#893806: tiff: CVE-2018-8905: heap-based buffer overflow in LZWDecodeCompat
Hi Salvatore, > > I have successfully reproduced this issue in latest upstream master > > branch and Buster but couldn't reproduce it neither in Wheezy nor in > > Jessie or Stretch. > > > > I am going to take a closer look, try to prepare a patch and declare > > Wheezy, Jessie and Stretch unaffected if appropriate. > > Please do it only mark as not-affected if you can confirm the > vulneable source/issue is not there/why the issue is introduced later, > but not only based on reproducibility. Just wanted to state that, > although I think given you want to have a closer look, that implies > already. Sorry for my imprecision, with "take a closer look" I was meaning "I am going to investigate the issue and, if applicable, prove that these tiff versions are not affected by the issue". ;) Cheers, Hugo -- Hugo Lefeuvre (hle)|www.owl.eu.com 4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA signature.asc Description: PGP signature
Bug#893806: tiff: CVE-2018-8905: heap-based buffer overflow in LZWDecodeCompat
Hi Hugo, On Sun, Apr 15, 2018 at 03:57:50PM -0400, Hugo Lefeuvre wrote: > Hi, > > I have successfully reproduced this issue in latest upstream master > branch and Buster but couldn't reproduce it neither in Wheezy nor in > Jessie or Stretch. > > I am going to take a closer look, try to prepare a patch and declare > Wheezy, Jessie and Stretch unaffected if appropriate. Please do it only mark as not-affected if you can confirm the vulneable source/issue is not there/why the issue is introduced later, but not only based on reproducibility. Just wanted to state that, although I think given you want to have a closer look, that implies already. Thanks for your work, Salvatore
Bug#893806: tiff: CVE-2018-8905: heap-based buffer overflow in LZWDecodeCompat
Hi, I have successfully reproduced this issue in latest upstream master branch and Buster but couldn't reproduce it neither in Wheezy nor in Jessie or Stretch. I am going to take a closer look, try to prepare a patch and declare Wheezy, Jessie and Stretch unaffected if appropriate. Regards, Hugo -- Hugo Lefeuvre (hle)|www.owl.eu.com 4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA signature.asc Description: PGP signature
Bug#893806: tiff: CVE-2018-8905: heap-based buffer overflow in LZWDecodeCompat
Source: tiff Version: 4.0.9-1 Severity: important Tags: security upstream Forwarded: http://bugzilla.maptools.org/show_bug.cgi?id=2780 Hi, the following vulnerability was published for tiff. CVE-2018-8905[0]: | In LibTIFF 4.0.9, a heap-based buffer overflow occurs in the function | LZWDecodeCompat in tif_lzw.c via a crafted TIFF file, as demonstrated | by tiff2ps. If you fix the vulnerability please also make sure to include the CVE (Common Vulnerabilities & Exposures) id in your changelog entry. For further information see: [0] https://security-tracker.debian.org/tracker/CVE-2018-8905 https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-8905 [1] http://bugzilla.maptools.org/show_bug.cgi?id=2780 Please adjust the affected versions in the BTS as needed. There is a poc file attached to the upstream bug [1] which can be used to verify a fix; the poc might not trigger but still the issue might be present in other versions than 4.0.9. There is not upstream commit yet which might help pinpointing then the issue. Regards, Salvatore
