From 8016f670918954483beb32a162441c0230127956 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Sun, 11 Sep 2022 15:10:04 +0100 Subject: [PATCH 1/2] Regression test for https://github.com/Exiv2/exiv2/issues/2352 --- test/data/issue_2352_poc.jpg | Bin 0 -> 20 bytes tests/bugfixes/github/test_issue_2352.py | 37 ++++++++++++++++++ .../test_regression_allfiles.py | 1 + 3 files changed, 38 insertions(+) create mode 100644 test/data/issue_2352_poc.jpg create mode 100644 tests/bugfixes/github/test_issue_2352.py diff --git a/test/data/issue_2352_poc.jpg b/test/data/issue_2352_poc.jpg new file mode 100644 index 0000000000000000000000000000000000000000..fb802ca5ee09aa2241c1cf1c4966f2adaa02e0b9 GIT binary patch literal 20 acmebEWzb?^U|^fX#=s4v8BYXmR{#JU3IksN literal 0 HcmV?d00001 diff --git a/tests/bugfixes/github/test_issue_2352.py b/tests/bugfixes/github/test_issue_2352.py new file mode 100644 index 0000000000..c48b86e0ca --- /dev/null +++ b/tests/bugfixes/github/test_issue_2352.py @@ -0,0 +1,37 @@ +# -*- coding: utf-8 -*- + +from system_tests import CaseMeta, check_no_ASAN_UBSAN_errors + +class issue_2352_floatToRationalCast_integer_overflow(metaclass=CaseMeta): + url = "https://github.com/Exiv2/exiv2/issues/2352" + filename = "$data_path/issue_2352_poc.jpg" + commands = ["$exiv2 -q $filename"] + retval = [0] + stderr = [""] + stdout = ["""File name : $filename +File size : 20 Bytes +MIME type : image/tiff +Image size : 0 x 0 +Thumbnail : None +Camera make : +Camera model : +Image timestamp : +File number : +Exposure time : +Aperture : +Exposure bias : +Flash : +Flash bias : +Focal length : +Subject distance: -214748.50 m +ISO speed : +Exposure mode : +Metering mode : +Macro mode : +Image quality : +White balance : +Copyright : +Exif comment : + +"""] + diff --git a/tests/regression_tests/test_regression_allfiles.py b/tests/regression_tests/test_regression_allfiles.py index b1dce5fa61..cbc2088c98 100644 --- a/tests/regression_tests/test_regression_allfiles.py +++ b/tests/regression_tests/test_regression_allfiles.py @@ -97,6 +97,7 @@ def get_valid_files(data_dir): "issue_2270_poc.webp", "issue_2320_poc.jpg", "issue_2339_poc.tiff", + "issue_2352_poc.jpg", "issue_ghsa_583f_w9pm_99r2_poc.jp2", "issue_ghsa_7569_phvm_vwc2_poc.jp2", "issue_ghsa_mxw9_qx4c_6m8v_poc.jp2", From ca20c9444757f578ef093a99fd15e44e68850d4a Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Sun, 11 Sep 2022 15:11:15 +0100 Subject: [PATCH 2/2] Credit to OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=51241 Fix bounds checking bug. --- src/types.cpp | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/types.cpp b/src/types.cpp index daf6e7a4e8..9b9e57309b 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -611,24 +611,22 @@ Rational parseRational(const std::string& s, bool& ok) { } Rational floatToRationalCast(float f) { - // Convert f to double because it simplifies the "in_range" check - // below. (INT_MAX can be represented accurately as a double, but - // gets rounded when it's converted to float.) + // Convert f to double because it simplifies the range checks + // below. (All int values can be losslessly converted to double, but + // sometimes get rounded when converted to float.) const double d = f; - // Don't allow INT_MIN (0x80000000) because it can cause a UBSAN failure in std::gcd(). - const bool in_range = std::numeric_limits::min() < d && d <= std::numeric_limits::max(); - if (!in_range) { - return {d > 0 ? 1 : -1, 0}; - } // Beware: primitive conversion algorithm - int32_t den = 1000000; - const auto d_as_int32_t = static_cast(d); - if (Safe::abs(d_as_int32_t) > 21474836) { - den = 1; - } else if (Safe::abs(d_as_int32_t) > 214748) { - den = 100; - } else if (Safe::abs(d_as_int32_t) > 2147) { + int32_t den; + if (std::fabs(d) <= std::numeric_limits::max() / 1000000) { + den = 1000000; + } else if (std::fabs(d) <= std::numeric_limits::max() / 10000) { den = 10000; + } else if (std::fabs(d) <= std::numeric_limits::max() / 100) { + den = 100; + } else if (std::fabs(d) <= std::numeric_limits::max()) { + den = 1; + } else { + return {d > 0 ? 1 : -1, 0}; } const auto nom = static_cast(std::round(d * den)); const int32_t g = std::gcd(nom, den);