Make XMP SDK code match scanf/printf format strings and types. #2671
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2671 +/- ##
=======================================
Coverage 63.92% 63.92%
=======================================
Files 103 103
Lines 22305 22305
Branches 10795 10795
=======================================
Hits 14258 14258
Misses 5826 5826
Partials 2221 2221 |
The numerical OutProc*(num) macros fill in their arguments via snprintf's "..." varargs part, and the type at the use site depends on the passed-in types. This might cause wrong types on the stack that cause undefined behavior in the snprintf() function, and reading past memory, outputting garbage. static_cast<> the macro arguments to the types matching the format string, to get the expected type width on the stack.
This function uses sscanf() with "%lld" or "%llx" to parse a string to an integer. However, nothing guarantees that sizeof(XMP_Int64) == sizeof(long long); the latter is what sscanf will fill with either of these format strings, to avoid memory corruption. Use a long long to match the sscanf size, and let the return statement cast it to XMP_Int64.
mandree
force-pushed
the
mandree-fix-xmp-types-formats
branch
from
9f8dd8c to
7ef6c3e
Compare
|
Thanks, but I'd rather we bumped the SDK itself first before pouring developer time into the old version... |
Of course. The thing is I needed to do something on FreeBSD (which is a multi-arch distribution) and I did not want those fixes to get lost in the rubble, but we can turn them into bug reports instead. I am too remote from this Adobe XMP SDK stuff to know where to look, in this case, I have a packager's view and am merely a consumer. |
|
The last time I looked at https://github.com/adobe/XMP-Toolkit-SDK, I couldn't figure out how to build it. This issue has some context: #1810. It would be great if somebody could figure out how to include it as a dependency rather than having a copy of it in our own repo. |
Yep. In the meantime, I suggest @mandree perhaps handles this as a local packaging patch, or downgrade these to warnings - they don't throw errors on other platforms AFAIK... |
|
Oh, no worries about keeping them as packaging patches in the FreeBSD ports collection, we will do that -- but when the type widths of varargs arguments and format strings do not match, crashes and corruptions of stack or heap and of unrelated variables can happen, especially for the *scanf() family of functions. These are very hard to find issues, because usually the crash site or malfunction site can not be traced back to the actual bug site easily -- and a format string type warning from any system is reason to fix the types once and for all. You really do not want to leave this unfixed. You have been warned. See the inttypes.h documentation, because since C99, there are fixed-width integer format strings, see
--however I would not bet anything valuable on their portability outside the well-maintained Unix/Linux systems. Although long long is C99 or some local extension, strictly speaking. Final word, do as you please, but until the day a revised and bugfixed Adobe XMP SDK can safely be kept separate (external), consider fixing the copy in exiv2 in the interim, however useless that may seem. Remember that the absence of a warning, or haphazardly well-behaved code does not mean the absence of a bug. The next compiler version changes layout and then boom. I've seen that happen too many times in the 30+ years that I've been around C code. |
|
Reviewing #1810 comments, I suggest to fix those issues locally in the copy, if Adobe seem to have abandoned the XMP SDK. |
No description provided.