Remedy CVE-2018-12913

[iibclothier]

This was originally reported in #90 which was closed without any remediation.

This can be reproduced by downloading the POC from here:
https://github.com/Edward-L/my-cve-list/blob/master/miniz/dos-an-infinite-loop-miniz_tinfl-c-398.poc

Uncomment the assert in the miniz_tinfl.c line 494, build the project.

And then run the example3.exe as shown with debugger:

example3.exe d, "C:\Temp\dos-an-infinite-loop-miniz_tinfl-c-398.poc" "C:\Temp\Bad.txt"

Because the POC file used as the input is not a valid zip file, we should not be attempting to extract it. However, as a guard against other zip files that may be maliciously crafted, we've added the condition and throw an error if both sym2 and counter are zero.

[iibclothier]

Apparently it was necessary to bump from version 1 to version 4 as per GitHub's deprecation of version 1 and 2 for the fuzzer. I ran to verify that the fuzzing could run successfully: https://github.com/iibclothier/miniz/actions/runs/12528459502

[sezero]

After this patch the following code is failing:

#include <stdlib.h>
#include <stdio.h>
#include "miniz_tinfl.h"

extern const unsigned char vkquake_pak[];
extern const int		   vkquake_pak_size;
extern const int		   vkquake_pak_decompressed_size;

unsigned char *extract_it (void)
{
	unsigned char	*vkquake_pak_extracted;

			size_t vkquake_pak_size_compressed = vkquake_pak_size, vkquake_pak_size_extracted = vkquake_pak_decompressed_size;

				tinfl_decompressor inflator;
				tinfl_init (&inflator);
				vkquake_pak_extracted = (unsigned char *) malloc (vkquake_pak_size_extracted);
				if (TINFL_STATUS_DONE != tinfl_decompress (
											 &inflator, vkquake_pak, &vkquake_pak_size_compressed, vkquake_pak_extracted, vkquake_pak_extracted,
											 &vkquake_pak_size_extracted, TINFL_FLAG_USING_NON_WRAPPING_OUTPUT_BUF))
				{
					fprintf (stderr, "Error extracting embedded pack\n");
					free(vkquake_pak_extracted);
					vkquake_pak_extracted = NULL;
				}
				return vkquake_pak_extracted;
}

int main (void) {
  void *p = extract_it();
  if (!p) return -1;
  free(p);
  return 0;
}

Is our code is at fault?

The embedded data is in the attached zip.

vk.zip

[iibclothier]

I can confirm that this fails on the changed code. Furthermore, the fuzzing also failed on this branch, despite it passing earlier on my fork. This suggests there may be legitimate scenarios where sym2 and counter can be both zero and not be malicious. The check will need to be more sophisticated beyond simply checking the two variables to guard against the malicious infinite loop.

Open to suggestions.

[sezero]

@uroni ?

[jsummers]

I tried to figure this out, but only made a little progress.

I see there's a variable named "code_len" that sometimes gets set to 0. That's suspicious, because zero-length codes don't exist in Deflate. I think it can only happen if something failed earlier that could have been detected then.

The first problem shown by the "poc" file is that miniz seems to neither (1) validate the "code length codes" table, nor (2) behave sanely if it is invalid. I'd prefer to do (2), but at this point I don't know how. I patched my copy of miniz to do (1), and I offer the patch, for what it's worth:

jsummers/deark@3f03e3d

I suspect it can be done more simply than this, by someone who understands miniz's internals.

There's a chance this fixes the whole problem, but I doubt it.