From 90434bbe2f1bc9c119b8ca1efb9235378c1acaea Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Wed, 22 Sep 2021 23:54:44 +0100 Subject: [PATCH 1/4] Regression test for: https://github.com/Exiv2/exiv2/issues/1920 --- test/data/issue_1920_poc.tiff | Bin 0 -> 96 bytes tests/bugfixes/github/test_issue_1920.py | 17 +++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 test/data/issue_1920_poc.tiff create mode 100644 tests/bugfixes/github/test_issue_1920.py diff --git a/test/data/issue_1920_poc.tiff b/test/data/issue_1920_poc.tiff new file mode 100644 index 0000000000000000000000000000000000000000..db19e854bc7850023c1e1c628612be700d541185 GIT binary patch literal 96 zcmebEWzb?^VBl2%0R}Ls@c%!UX5?pJ0!k?Wv4H|u2q+QY>KEb|p}_DTg#Q0OW&+fO Oq6TCP8(1R?NDlz1gA@+{ literal 0 HcmV?d00001 diff --git a/tests/bugfixes/github/test_issue_1920.py b/tests/bugfixes/github/test_issue_1920.py new file mode 100644 index 0000000000..05ebf280e4 --- /dev/null +++ b/tests/bugfixes/github/test_issue_1920.py @@ -0,0 +1,17 @@ +# -*- coding: utf-8 -*- + +from system_tests import CaseMeta, path, check_no_ASAN_UBSAN_errors + +class PentaxMakerNotePrintTimeSignedLeftShift(metaclass=CaseMeta): + """ + Regression test for the bug described in: + https://github.com/Exiv2/exiv2/issues/1920 + """ + url = "https://github.com/Exiv2/exiv2/issues/1920" + + filename = path("$data_path/issue_1920_poc.tiff") + commands = ["$exiv2 -q -Pt $filename"] + stderr = [""] + retval = [0] + + compare_stdout = check_no_ASAN_UBSAN_errors From a71bb64fe959c0fe891de5cec99bb6ca44172818 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Wed, 22 Sep 2021 23:55:15 +0100 Subject: [PATCH 2/4] Credit to OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=39060 Fix UBSAN failure caused by left shift of a negative number. --- src/pentaxmn_int.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pentaxmn_int.cpp b/src/pentaxmn_int.cpp index f4cb3863f8..32d6f30548 100644 --- a/src/pentaxmn_int.cpp +++ b/src/pentaxmn_int.cpp @@ -1036,7 +1036,7 @@ namespace Exiv2 { std::ostream& PentaxMakerNote::printDate(std::ostream& os, const Value& value, const ExifData*) { /* I choose same format as is used inside EXIF itself */ - os << ((value.toLong(0) << 8) + value.toLong(1)); + os << ((static_cast(value.toLong(0)) << 8) + value.toLong(1)); os << ":"; os << std::setw(2) << std::setfill('0') << value.toLong(2); os << ":"; From fc07f1864405aca26c63d3d8fcbc28f34725c8dc Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Thu, 23 Sep 2021 09:54:59 +0100 Subject: [PATCH 3/4] Add CodeQL query to detect variants of issue #1920. --- .../exiv2-cpp-queries/signed_shift.ql | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .github/codeql-queries/exiv2-cpp-queries/signed_shift.ql diff --git a/.github/codeql-queries/exiv2-cpp-queries/signed_shift.ql b/.github/codeql-queries/exiv2-cpp-queries/signed_shift.ql new file mode 100644 index 0000000000..a33ec8d29b --- /dev/null +++ b/.github/codeql-queries/exiv2-cpp-queries/signed_shift.ql @@ -0,0 +1,24 @@ +/** + * @name Signed shift + * @description Shifting a negative number is undefined behavior, + * so it is risky to shift a signed number. + * @kind problem + * @problem.severity warning + * @id cpp/signed-shift + * @tags security + * external/cwe/cwe-758 + */ + +// See the "Bitwise shift operators" section here: +// https://en.cppreference.com/w/cpp/language/operator_arithmetic +import cpp +import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis + +from BinaryBitwiseOperation shift, Expr lhs +where + (shift instanceof LShiftExpr or shift instanceof RShiftExpr) and + lhs = shift.getLeftOperand().getFullyConverted() and + lowerBound(lhs) < 0 +select shift, + "This signed shift could cause undefined behavior if the value is negative. Type of lhs: " + + lhs.getType().toString() From 83b9944cdda265ce9ede4c0c749ea5497c18415e Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Mon, 4 Oct 2021 10:09:11 +0100 Subject: [PATCH 4/4] We only need to extract one byte. --- src/pentaxmn_int.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pentaxmn_int.cpp b/src/pentaxmn_int.cpp index 32d6f30548..a088ff4e13 100644 --- a/src/pentaxmn_int.cpp +++ b/src/pentaxmn_int.cpp @@ -1036,7 +1036,7 @@ namespace Exiv2 { std::ostream& PentaxMakerNote::printDate(std::ostream& os, const Value& value, const ExifData*) { /* I choose same format as is used inside EXIF itself */ - os << ((static_cast(value.toLong(0)) << 8) + value.toLong(1)); + os << ((static_cast(value.toLong(0)) << 8) + value.toLong(1)); os << ":"; os << std::setw(2) << std::setfill('0') << value.toLong(2); os << ":";