Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NULL pointer derefence and assertion failure with invalid inputs #36

Closed
4 tasks done
psychon opened this issue Nov 30, 2017 · 4 comments
Closed
4 tasks done

NULL pointer derefence and assertion failure with invalid inputs #36

psychon opened this issue Nov 30, 2017 · 4 comments
Assignees
Labels
Milestone

Comments

@psychon
Copy link

psychon commented Nov 30, 2017

Hi,

I just started experimenting with american fuzzy lop and picked charls as my target. I wrote the following code to feed data to CharLS:

#include "src/charls.h"

#include <unistd.h>
#include <fcntl.h>
#include <string.h>

static char result[1024 * 1024];
static char input[1024 * 1024];

int main(int argc, char *argv[])
{
	int fd = 0;
	if (argc == 2)
	{
		fd = open(argv[1], O_RDONLY);
		if (fd < 0) {
			fprintf(stderr, "Failed to open '%s': %s\n",
					argv[1], strerror(errno));
			return 1;
		}
	}

	size_t input_length = read(fd, input, sizeof(input));
	std::cout << (int) JpegLsDecode(result, sizeof(result), input, input_length, nullptr, nullptr) << std::endl;
	return 0;
}

Within seconds, afl managed to crash this program.

Valgrind result for first file https://filebin.ca/3j4hDmAVjSvI:

==16586== Invalid read of size 8
==16586==    at 0x4E4793: JpegStreamReader::Read(ByteStreamInfo) (jpegstreamreader.cpp:124)
==16586==    by 0x41042D: JpegLsDecodeStream (interface.cpp:163)
==16586==    by 0x41042D: JpegLsDecode (interface.cpp:216)
==16586==    by 0x40DC91: main (charlsfuzz.cpp:24)
==16586==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

The above seems to be JlsCodecFactory::CreateCodec returning a nullptr, which is something that JpegStreamReader::Read does not check for before dereferencing qcodec in line 124.

Plus, there are lots (currently 9) of files which cause the following assertion failure, one of them is https://filebin.ca/3j4hfYzIdJMm:

charlsfuzz: /tmp/charls/src/decoderstrategy.h:129: void DecoderStrategy::MakeValid(): Assertion `static_cast<size_t>(_validBits) <=bufType_bit_count - 8' failed.

Edit: Here is another failed assertion:
https://filebin.ca/3j4yTkNxVuQb

charlsfuzz: /tmp/charls/src/context.h:60: void JlsContext::UpdateVariables(int32_t, int32_t, int32_t): Assertion `std::abs(b) < 65536 * 256' failed.

Edit: https://filebin.ca/3j57Vrm3jPkZ

charlsfuzz: /tmp/charls/src/scan.h:239: SAMPLE JlsCodec<LosslessTraits<uint16_t, 16>, DecoderStrategy>::DoRegular(int32_t, int32_t, int32_t, DecoderStrategy *) [Traits = LosslessTraits<uint16_t, 16>, Strategy = DecoderStrategy]: Assertion `std::abs(ErrVal) < 65535' failed.

Edit: The above as a check-list since I see that some of these were already fixed (by the way, assuming I find more such "evil inputs", should I open one issue per file or continue grouping them like this? Should I open a new issue or keep updating this one?):

@vbaderks vbaderks added the bug label Nov 30, 2017
@vbaderks vbaderks self-assigned this Nov 30, 2017
@vbaderks
Copy link
Contributor

Thanks for reporting these issues. I have read about these kind of tools, but never have found the time to set them up for charls. JpegLsDecode should of course remain stable for invalid encoded data.

@vbaderks
Copy link
Contributor

I am trying to setup, american fuzzy lop, to be able to validate the fixes I am planing to make. The documentation of american fuzzy lop describes something of a minimum input file. Are you using a special file to start the fuzzy verification process?

@psychon
Copy link
Author

psychon commented Dec 13, 2017

For the fuzzing results above I used all the sample files that come with charls and asked afl-cmin to pick some subset of those. However, afl warned that the resulting files were all too large which meant that fuzzing would take long and likely have bad results. Still, afl found crashes in just seconds.
I think (but am not sure) that the last file from the results above already was produced in the following way instead.

To produce a minimum input file, I changed my charlsfuzz program into the following:

#include "src/charls.h"

#include <unistd.h>
#include <fcntl.h>
#include <string.h>

static char result[1024 * 1024];
static char input[1024 * 1024];

static void generate_once(size_t *bytesWritten)
{
	JlsParameters params;

	memset(&input[0], 0, sizeof(input));
	memset(&params, 0, sizeof(params));

	params.width = 1;
	params.height = 1;
	params.bitsPerSample = 8;
	params.stride = 0;
	params.components = 3;

	CharlsApiResultType i = JpegLsEncode(result, sizeof(result), bytesWritten, input, sizeof(input), &params, nullptr);
	if (i != charls::ApiResult::OK)
		fprintf(stderr, "Error while encoding: %d\n", static_cast<int>(i));
	i = JpegLsDecode(input, sizeof(input), result, *bytesWritten, nullptr, nullptr);
	if (i != charls::ApiResult::OK)
		fprintf(stderr, "Error while decoding: %d\n", static_cast<int>(i));
}

int main(int argc, char *argv[])
{
	size_t bytesWritten;
	int fd = 0;
	if (argc == 2)
	{
		if (argv[1][0] == '\0') {
			// Write some small-ish JPEG-LS file to stdout
			generate_once(&bytesWritten);
			write(1, result, bytesWritten);
			return 0;
		}

		fd = open(argv[1], O_RDONLY);
		if (fd < 0) {
			fprintf(stderr, "Failed to open '%s': %s\n",
					argv[1], strerror(errno));
			return 1;
		}
	}

	generate_once(&bytesWritten);

	__AFL_INIT();

	while (__AFL_LOOP(100))
	{
		memset(&input[0], 0, sizeof(input));
		size_t input_length = read(fd, input, sizeof(input));
		JpegLsDecode(result, sizeof(result), input, input_length, nullptr, nullptr);
	}
	return 0;
}

When called with an empty argument, this writes a small JPEG-LS file to stdout. (It might make sense to also produce some input files with different parameters; dunno how much of a difference this really makes to charls; perhaps feeding afl with files with different interleave modes makes sense?)
Thus, I used something like the following for later fuzzing runs:

$ mkdir input
$ ./charlsfuzz '' > input
$ afl-fuzz -i input -o output ./charlsfuzz

Another (unrelated) change compared to my original program is the use of the __AFL_* macros. This is clang-mode, meaning that instead of afl-g++, everything must now be compiled with afl-clang-fast++. Normally, afl fork()s one process per input file, which takes some time. With the above, all the code up to __AFL_INIT() is executed before forking, which saves a tiny amount of time, and a forked process is re-used for 100 input files instead of just one (that's the __AFL_LOOP).
The result is an immense increase of performance. Previously afl could run around 1k inputs per second and with __AFL_LOOP it can now run 10k inputs per second.

@vbaderks vbaderks added this to the 2.2.0 milestone Jul 14, 2019
vbaderks added a commit that referenced this issue Feb 20, 2020
The NEAR parameter needs to be in the range [0, min(255, max-sample-value)]. This needs to be verified to ensure other pre-conditions are not broken.

Note: found with the american fuzzy lop tool
vbaderks added a commit that referenced this issue Feb 20, 2020
The NEAR parameter needs to be in the range [0, min(255, max-sample-value)]. This needs to be verified to ensure other pre-conditions are not broken.

Note: found with the american fuzzy lop tool
vbaderks added a commit that referenced this issue Feb 21, 2020
To make it possible to use the american fuzzy lop (afl) fuzzing framework, add a test application that can be used to fuzz the decoding process.
vbaderks added a commit that referenced this issue Feb 21, 2020
It is not valid if a Start Of Scan (SOS) marker is found before a Start of Frame (SOF)
Rename error code 16 to make it possible to use the error in more places.
vbaderks added a commit that referenced this issue Feb 29, 2020
A valid JPEG-LS stream can have only 1 Start of Frame (SOF) marker segment. Add a unit test + fix to ensure duplicate sofs are detected.
@vbaderks
Copy link
Contributor

vbaderks commented Jan 3, 2021

With 6a49aea in place, american fuzzy lop 2.59d doesn't find any crashes (8 hours running)

@vbaderks vbaderks closed this as completed Jan 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants