Discussion:
[Radiance-general] crashes in radiance from fuzzers
j***@gmail.com
2018-07-22 12:24:32 UTC
Permalink
Hello all,

I maintain an image processing library that supports HDR files. We've
been fuzzing it to find bugs, and HDR is one of the ones that seems
vulnerable to attack.

The obvious one is the globmatch() function in header.c -- during file
read, it expands into a 64-byte buffer here:

https://radiance-online.org/cgi-bin/viewcvs.cgi/ray/src/common/header.c?view=markup#l231

It's easy to make an HDR file which would overwrite this and let an
attacker take control of the user's machine (I think), since that
buffer is usually allocated on the stack.

Does this sound like something that should be fixed? I could try to
cook up a patch, if it would be useful.

John
Greg Ward
2018-07-22 13:25:30 UTC
Permalink
Hi John,

There are many ways to overrun buffers in Radiance. It's not designed as a hardened system, and the general assumption is that it isn't running it in a privileged mode. It would take a little time and effort to patch the possible attack routes just in the header-reading code, including:

globmatch() - needs check as you say for output buffer length
formatval() - typically called with a fixed-size buffer in first argument, which is easily overrun by a too-long format string in the HDR file
oldreadcolrs() - needs a check for malicious run-length codes

If you're only concerned about the HDR input side of things, this might be enough. If you enlarge it to all Radiance input types, we have a long road ahead....

Cheers,
-Greg
Date: July 22, 2018 8:24:32 AM EDT
Hello all,
I maintain an image processing library that supports HDR files. We've
been fuzzing it to find bugs, and HDR is one of the ones that seems
vulnerable to attack.
The obvious one is the globmatch() function in header.c -- during file
https://radiance-online.org/cgi-bin/viewcvs.cgi/ray/src/common/header.c?view=markup#l231
It's easy to make an HDR file which would overwrite this and let an
attacker take control of the user's machine (I think), since that
buffer is usually allocated on the stack.
Does this sound like something that should be fixed? I could try to
cook up a patch, if it would be useful.
John
j***@gmail.com
2018-07-22 16:56:24 UTC
Permalink
Post by j***@gmail.com
The obvious one is the globmatch() function in header.c -- during file
https://radiance-online.org/cgi-bin/viewcvs.cgi/ray/src/common/header.c?view=markup#l231
Actually, thinking about it again, just changing that 64 into MAXLINE
should stop this problem at least. The read routines guarantee that
lines can't be longer than MAXLINE, so as long as fs[] is the same
size or bigger, it can't overflow. It will increase the amount of
stack radiance needs, unfortunately.

That's the change I've made in my lib anyway. If the fuzzers find
anything else, I'll post a note here.

John
Greg Ward
2018-08-02 18:36:19 UTC
Permalink
Hi John,

I just implemented a check in formatval() against a new #define MAXFMTLEN that should prevent buffer overruns from Radiance header input.

Cheers,
-Greg
Date: July 22, 2018 9:56:24 AM PDT
Post by j***@gmail.com
The obvious one is the globmatch() function in header.c -- during file
https://radiance-online.org/cgi-bin/viewcvs.cgi/ray/src/common/header.c?view=markup#l231
Actually, thinking about it again, just changing that 64 into MAXLINE
should stop this problem at least. The read routines guarantee that
lines can't be longer than MAXLINE, so as long as fs[] is the same
size or bigger, it can't overflow. It will increase the amount of
stack radiance needs, unfortunately.
That's the change I've made in my lib anyway. If the fuzzers find
anything else, I'll post a note here.
John
j***@gmail.com
2018-08-03 17:32:54 UTC
Permalink
Thanks Greg!

I made the matching change in my loader:

https://github.com/jcupitt/libvips/commit/0b3565c04d7b3f491126433cd42edeb0618824b6
Post by Greg Ward
Hi John,
I just implemented a check in formatval() against a new #define MAXFMTLEN that should prevent buffer overruns from Radiance header input.
Cheers,
-Greg
Date: July 22, 2018 9:56:24 AM PDT
Post by j***@gmail.com
The obvious one is the globmatch() function in header.c -- during file
https://radiance-online.org/cgi-bin/viewcvs.cgi/ray/src/common/header.c?view=markup#l231
Actually, thinking about it again, just changing that 64 into MAXLINE
should stop this problem at least. The read routines guarantee that
lines can't be longer than MAXLINE, so as long as fs[] is the same
size or bigger, it can't overflow. It will increase the amount of
stack radiance needs, unfortunately.
That's the change I've made in my lib anyway. If the fuzzers find
anything else, I'll post a note here.
John
_______________________________________________
Radiance-general mailing list
https://www.radiance-online.org/mailman/listinfo/radiance-general
Loading...