Fix UBSAN failure caused by left-shift of negative number #1921
Conversation
…l?id=39060 Fix UBSAN failure caused by left shift of a negative number.
kevinbackhouse
added
bug
OSS-Fuzz
Codecov Report
@@ Coverage Diff @@
## main #1921 +/- ##
==========================================
- Coverage 61.13% 61.12% -0.01%
==========================================
Files 96 96
Lines 19051 19068 +17
Branches 9729 9745 +16
==========================================
+ Hits 11647 11656 +9
- Misses 5090 5093 +3
- Partials 2314 2319 +5
Continue to review full report at Codecov.
|
kevinbackhouse
force-pushed
the
FixIssue1920
branch
from
23314b3
to
fc07f18
Compare
|
In the third commit, I added a CodeQL query which warns about other signed shifts in the codebase. It has generated quite a few warnings, which I plan to fix in a separate PR, so that the warnings show up as "closed" on the Security tab. |
src/pentaxmn_int.cpp
Outdated
| @@ -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<uint64_t>(value.toLong(0)) << 8) + value.toLong(1)); | |||
There was a problem hiding this comment.
uint32_t doesn't cut it here?
There was a problem hiding this comment.
Actually uint16_t is probably sufficient for the intended purpose here. I think this is probably a similar scenario to #1706. The value is supposed to be a byte, but there's no validation on that, so a malicious file can contain a long instead. For the valid cases, we only need to extract the byte.
I'll change it.
|
I can put the CodeQL query in a separate PR if you prefer. It has generated a lot of spam in the diff. |
Fixes: #1920
Credit to OSS-Fuzz: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=39060
Add a cast to
uint64_tto fix a UBSAN failure.