Skip to content

Commit 6aa11df

Browse files
committed
Improve deep recursion detection in exif_data_load_data_content.
The existing detection was still vulnerable to pathological cases causing DoS by wasting CPU. The new algorithm takes the number of tags into account to make it harder to abuse by cases using shallow recursion but with a very large number of tags. This improves on commit 5d28011 which wasn't sufficient to counter this kind of case. The limitation in the previous fix was discovered by Laurent Delosieres, Secunia Research at Flexera (Secunia Advisory SA84652) and is assigned the identifier CVE-2018-20030.
1 parent 7c82ed9 commit 6aa11df

File tree

2 files changed

+38
-8
lines changed

2 files changed

+38
-8
lines changed

Diff for: NEWS

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ libexif-0.6.x:
33
* Updated translations for most languages
44
* Fixed C89 compatibility
55
* Fixed warnings on recent versions of autoconf
6+
* Fix for recursion DoS CVE-2018-20030
67

78
libexif-0.6.21 (2012-07-12):
89
* New translations: en_AU, uk

Diff for: libexif/exif-data.c

+37-8
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include <libexif/olympus/exif-mnote-data-olympus.h>
3636
#include <libexif/pentax/exif-mnote-data-pentax.h>
3737

38+
#include <math.h>
3839
#include <stdlib.h>
3940
#include <stdio.h>
4041
#include <string.h>
@@ -350,20 +351,34 @@ if (data->ifd[(i)]->count) { \
350351
break; \
351352
}
352353

354+
/*! Calculate the recursion cost added by one level of IFD loading.
355+
*
356+
* The work performed is related to the cost in the exponential relation
357+
* work=1.1**cost
358+
*/
359+
static unsigned int
360+
level_cost(unsigned int n)
361+
{
362+
static const double log_1_1 = 0.09531017980432493;
363+
364+
/* Adding 0.1 protects against the case where n==1 */
365+
return ceil(log(n + 0.1)/log_1_1);
366+
}
367+
353368
/*! Load data for an IFD.
354369
*
355370
* \param[in,out] data #ExifData
356371
* \param[in] ifd IFD to load
357372
* \param[in] d pointer to buffer containing raw IFD data
358373
* \param[in] ds size of raw data in buffer at \c d
359374
* \param[in] offset offset into buffer at \c d at which IFD starts
360-
* \param[in] recursion_depth number of times this function has been
361-
* recursively called without returning
375+
* \param[in] recursion_cost factor indicating how expensive this recursive
376+
* call could be
362377
*/
363378
static void
364379
exif_data_load_data_content (ExifData *data, ExifIfd ifd,
365380
const unsigned char *d,
366-
unsigned int ds, unsigned int offset, unsigned int recursion_depth)
381+
unsigned int ds, unsigned int offset, unsigned int recursion_cost)
367382
{
368383
ExifLong o, thumbnail_offset = 0, thumbnail_length = 0;
369384
ExifShort n;
@@ -378,9 +393,20 @@ exif_data_load_data_content (ExifData *data, ExifIfd ifd,
378393
if ((((int)ifd) < 0) || ( ((int)ifd) >= EXIF_IFD_COUNT))
379394
return;
380395

381-
if (recursion_depth > 12) {
396+
if (recursion_cost > 170) {
397+
/*
398+
* recursion_cost is a logarithmic-scale indicator of how expensive this
399+
* recursive call might end up being. It is an indicator of the depth of
400+
* recursion as well as the potential for worst-case future recursive
401+
* calls. Since it's difficult to tell ahead of time how often recursion
402+
* will occur, this assumes the worst by assuming every tag could end up
403+
* causing recursion.
404+
* The value of 170 was chosen to limit typical EXIF structures to a
405+
* recursive depth of about 6, but pathological ones (those with very
406+
* many tags) to only 2.
407+
*/
382408
exif_log (data->priv->log, EXIF_LOG_CODE_CORRUPT_DATA, "ExifData",
383-
"Deep recursion detected!");
409+
"Deep/expensive recursion detected!");
384410
return;
385411
}
386412

@@ -422,15 +448,18 @@ exif_data_load_data_content (ExifData *data, ExifIfd ifd,
422448
switch (tag) {
423449
case EXIF_TAG_EXIF_IFD_POINTER:
424450
CHECK_REC (EXIF_IFD_EXIF);
425-
exif_data_load_data_content (data, EXIF_IFD_EXIF, d, ds, o, recursion_depth + 1);
451+
exif_data_load_data_content (data, EXIF_IFD_EXIF, d, ds, o,
452+
recursion_cost + level_cost(n));
426453
break;
427454
case EXIF_TAG_GPS_INFO_IFD_POINTER:
428455
CHECK_REC (EXIF_IFD_GPS);
429-
exif_data_load_data_content (data, EXIF_IFD_GPS, d, ds, o, recursion_depth + 1);
456+
exif_data_load_data_content (data, EXIF_IFD_GPS, d, ds, o,
457+
recursion_cost + level_cost(n));
430458
break;
431459
case EXIF_TAG_INTEROPERABILITY_IFD_POINTER:
432460
CHECK_REC (EXIF_IFD_INTEROPERABILITY);
433-
exif_data_load_data_content (data, EXIF_IFD_INTEROPERABILITY, d, ds, o, recursion_depth + 1);
461+
exif_data_load_data_content (data, EXIF_IFD_INTEROPERABILITY, d, ds, o,
462+
recursion_cost + level_cost(n));
434463
break;
435464
case EXIF_TAG_JPEG_INTERCHANGE_FORMAT:
436465
thumbnail_offset = o;

0 commit comments

Comments
 (0)