|
 |
3f58c5 |
From 1e07c98dfcbd8ac10ee02088f08235f5e1700148 Mon Sep 17 00:00:00 2001
|
|
|
3f58c5 |
From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= <dan.cermak@cgc-instruments.com>
|
|
|
3f58c5 |
Date: Wed, 27 Sep 2017 23:38:49 +0200
|
|
|
3f58c5 |
Subject: Fixed wrong brackets: size*count + pad can overflow before the cast
|
|
|
3f58c5 |
|
|
|
3f58c5 |
=> Should fix #76 (most of the work has been done by Robin Mills in
|
|
|
3f58c5 |
6e3855aed7ba8bb4731fc4087ca7f9078b2f3d97)
|
|
|
3f58c5 |
|
|
|
3f58c5 |
The problem with #76 is the contents of the 26th IFD, with the
|
|
|
3f58c5 |
following contents:
|
|
|
3f58c5 |
tag: 0x8649
|
|
|
3f58c5 |
type: 0x1
|
|
|
3f58c5 |
count: 0xffff ffff
|
|
|
3f58c5 |
offset: 0x4974
|
|
|
3f58c5 |
|
|
|
3f58c5 |
The issue is the size of count (uint32_t), as adding anything to it
|
|
|
3f58c5 |
causes an overflow. Especially the expression:
|
|
|
3f58c5 |
(size*count + pad+20)
|
|
|
3f58c5 |
results in an overflow and gives 20 as a result instead of
|
|
|
3f58c5 |
0x100000014, thus the condition in the if in the next line is false
|
|
|
3f58c5 |
and the program continues to run (until it crashes at io.read).
|
|
|
3f58c5 |
|
|
|
3f58c5 |
To properly account for the overflow, the brackets have to be removed,
|
|
|
3f58c5 |
as then the result is saved in the correctly sized type and not cast
|
|
|
3f58c5 |
after being calculated in the smaller type.
|
|
|
3f58c5 |
|
|
|
3f58c5 |
diff --git a/src/image.cpp b/src/image.cpp
|
|
|
3f58c5 |
index ec5b873e..199671b9 100644
|
|
|
3f58c5 |
--- a/src/image.cpp
|
|
|
3f58c5 |
+++ b/src/image.cpp
|
|
|
3f58c5 |
@@ -401,7 +401,7 @@ namespace Exiv2 {
|
|
|
3f58c5 |
// if ( offset > io.size() ) offset = 0; // Denial of service?
|
|
|
3f58c5 |
|
|
|
3f58c5 |
// #55 memory allocation crash test/data/POC8
|
|
|
3f58c5 |
- long long allocate = (long long) (size*count + pad+20);
|
|
|
3f58c5 |
+ long long allocate = (long long) size*count + pad+20;
|
|
|
3f58c5 |
if ( allocate > (long long) io.size() ) {
|
|
|
3f58c5 |
throw Error(57);
|
|
|
3f58c5 |
}
|