Skip to content

Commit

Permalink
Avoid overflow in ljpeg_start().
Browse files Browse the repository at this point in the history
  • Loading branch information
@abrander
abrander committed May 11, 2015
1 parent 6eabf1f commit 983bda1
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion plugins/load-dcraw/dcraw.cc
View file
Expand Up @@ -890,7 +890,8 @@ struct jhead {

int CLASS ljpeg_start (struct jhead *jh, int info_only)
{
int c, tag, len;
int c, tag;
ushort len;
uchar data[0x10000];
const uchar *dp;

Expand Down

1 comment on commit 983bda1

@abrander
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch was the one received from David Coffin (author of dcraw) off channel before making the vulnerability public if I remember correctly.

I think the len < 2 check is related to the line

len = (data[2] << 8 | data[3]) - 2;

where len can be USHORT-MAX-1, which may result in a possible read error in the following fread() call - but you can easily trigger a read error even without underflowing. I don't see any security related bugs triggered by omitting the check, len is an unsigned integer, and the data array is of size USHORT_MAX+1. Neither overflow nor underflow can occur.

The fix in netpbm and Fedora is somewhat cleaner than the upstream fix in dcraw, but all are good from a security perspective.

For the interested: Rawstudio plans to remove dcraw completely from a future version and rely solely on RawSpeed - and maybe calling an external dcraw binary as a fall-back option to support ancient file formats.

Please sign in to comment.