From d9bc087ea99597c27112110bc05222e3c7cfb629 Mon Sep 17 00:00:00 2001 From: Matthias Andree Date: Sat, 1 Jul 2023 13:38:22 +0200 Subject: [PATCH 1/2] XMPMeta.cpp: match types to format strings in OutProc* macros 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. --- xmpsdk/src/XMPMeta.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/xmpsdk/src/XMPMeta.cpp b/xmpsdk/src/XMPMeta.cpp index 30ff4a5a75..9d25a750b7 100644 --- a/xmpsdk/src/XMPMeta.cpp +++ b/xmpsdk/src/XMPMeta.cpp @@ -81,17 +81,17 @@ static const char * kTenSpaces = " "; #define OutProcString(str) { status = (*outProc) ( refCon, (str).c_str(), (str).size() ); if ( status != 0 ) goto EXIT; } -#define OutProcULong(num) { snprintf ( buffer, sizeof(buffer), "%lu", (num) ); /* AUDIT: Using sizeof for snprintf length is safe */ \ +#define OutProcULong(num) { snprintf ( buffer, sizeof(buffer), "%lu", static_cast(num) ); /* AUDIT: Using sizeof for snprintf length is safe */ \ status = (*outProc) ( refCon, buffer, strlen(buffer) ); if ( status != 0 ) goto EXIT; } #ifdef __APPLE__ -#define OutProcHexInt(num) { snprintf ( buffer, sizeof(buffer), "%X", (num) ); /* AUDIT: Using sizeof for snprintf length is safe */ \ +#define OutProcHexInt(num) { snprintf ( buffer, sizeof(buffer), "%X", static_cast(num) ); /* AUDIT: Using sizeof for snprintf length is safe */ \ status = (*outProc) ( refCon, buffer, strlen(buffer) ); if ( status != 0 ) goto EXIT; } #else -#define OutProcHexInt(num) { snprintf ( buffer, sizeof(buffer), "%lX", (num) ); /* AUDIT: Using sizeof for snprintf length is safe */ \ +#define OutProcHexInt(num) { snprintf ( buffer, sizeof(buffer), "%lX", static_cast(num) ); /* AUDIT: Using sizeof for snprintf length is safe */ \ status = (*outProc) ( refCon, buffer, strlen(buffer) ); if ( status != 0 ) goto EXIT; } #endif -#define OutProcHexByte(num) { snprintf ( buffer, sizeof(buffer), "%.2X", (num) ); /* AUDIT: Using sizeof for snprintf length is safe */ \ +#define OutProcHexByte(num) { snprintf ( buffer, sizeof(buffer), "%.2X", static_cast(num) ); /* AUDIT: Using sizeof for snprintf length is safe */ \ status = (*outProc) ( refCon, buffer, strlen(buffer) ); if ( status != 0 ) goto EXIT; } static const char * kIndent = " "; From 7ef6c3ec45937f6a5d0ab81d161e80e64f072c4d Mon Sep 17 00:00:00 2001 From: Matthias Andree Date: Sat, 1 Jul 2023 13:41:26 +0200 Subject: [PATCH 2/2] Fix XMPUtils::ConvertToInt64 sscanf() type 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. --- xmpsdk/src/XMPUtils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmpsdk/src/XMPUtils.cpp b/xmpsdk/src/XMPUtils.cpp index 89afd3d62d..9f9ec0bb15 100644 --- a/xmpsdk/src/XMPUtils.cpp +++ b/xmpsdk/src/XMPUtils.cpp @@ -1215,7 +1215,7 @@ XMPUtils::ConvertToInt64 ( XMP_StringPtr strValue ) int count; char nextCh; - XMP_Int64 result; + long long result; if ( ! XMP_LitNMatch ( strValue, "0x", 2 ) ) { count = sscanf ( strValue, "%lld%c", &result, &nextCh );