From bb0c77e5bf617777390d6e0f1ed6152b908cbb87 Mon Sep 17 00:00:00 2001 From: Sergiy Kuryata Date: Fri, 13 Feb 2015 00:29:53 -0800 Subject: [PATCH] Fix sscanf implementation in PAL MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Implementation of sscanf was incorrectly handling cases where the input format was in the form of “%2c”, “%2s”, or “%[“ which could result in an access violation. The problem was that sscanf was calling sscanf_s with incorrect set of arguments. Implementation of sscanf_s expects to find the size of the target buffer in the list of its parameters but that parameter was not present in some cases. This change fixes this issue and re-enables previously disabled sscanf tests. --- src/pal/src/cruntime/printf.cpp | 40 ++++++++++++++-------- src/pal/tests/palsuite/paltestlist.txt | 4 +++ .../tests/palsuite/paltestlist_to_be_reviewed.txt | 4 --- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/pal/src/cruntime/printf.cpp b/src/pal/src/cruntime/printf.cpp index bd22af0..aed5904 100644 --- a/src/pal/src/cruntime/printf.cpp +++ b/src/pal/src/cruntime/printf.cpp @@ -1154,20 +1154,24 @@ int PAL_vsscanf(LPCSTR Buffer, LPCSTR Format, va_list ap) if (Store) { - // sscanf_s requires that if we are trying to read "%s" or "%c", then + // sscanf_s requires that if we are trying to read "%s" or "%c" or “%[“, then // the size of the buffer must follow the buffer we are trying to read into. - // - // QUES: Is there a better way to get the string buffer size for the buffer - // returned by va_arg below? If so, we should replace 1024 below with that. voidPtr = va_arg(ap, LPVOID); unsigned typeLen = 0; - if ((TempBuff[0] == '%') && (TempBuff[1] == 's' || TempBuff[1] == 'S')) + if ((Type == SCANF_TYPE_STRING) || (Type == SCANF_TYPE_BRACKETS)) { - typeLen = 1024; + // Since this is not a Safe CRT API we don’t really know the size of the destination + // buffer provided by the caller. So we have to assume that the caller has allocated + // enough space to hold either the width specified in the format or the entire input + // string plus ‘\0’. + typeLen = ((Width > 0) ? Width : strlen(Buffer)) + 1; } - else if ((TempBuff[0] == '%') && (TempBuff[1] == 'c' || TempBuff[1] == 'C')) + else if (Type == SCANF_TYPE_CHAR) { - typeLen = sizeof(CHAR); + // Check whether the format string contains number of characters + // that should be read from the input string. + // Note: ‘\0’ does not get appended in the “%c” case. + typeLen = (Width > 0) ? Width : 1; } if (typeLen > 0) @@ -1175,7 +1179,9 @@ int PAL_vsscanf(LPCSTR Buffer, LPCSTR Format, va_list ap) ret = sscanf_s(Buff, TempBuff, voidPtr, typeLen, &n); } else + { ret = sscanf_s(Buff, TempBuff, voidPtr, &n); + } } else { @@ -1390,17 +1396,21 @@ int PAL_wvsscanf(LPCWSTR Buffer, LPCWSTR Format, va_list ap) voidPtr = va_arg(ap, LPVOID); // sscanf_s requires that if we are trying to read "%s" or "%c", then // the size of the buffer must follow the buffer we are trying to read into. - // - // QUES: Is there a better way to get the string buffer size for the buffer - // returned by va_arg below? If so, we should replace 1024 below with that. unsigned typeLen = 0; - if ((TempBuff[0] == '%') && (TempBuff[1] == 's' || TempBuff[1] == 'S')) + if (Type == SCANF_TYPE_STRING) { - typeLen = 1024; + // We don’t really know the size of the destination buffer provided by the + // caller. So we have to assume that the caller has allocated enough space + // to hold either the width specified in the format or the entire input + // string plus ‘\0’. + typeLen = ((Width > 0) ? Width : PAL_wcslen(Buffer)) + 1; } - else if ((TempBuff[0] == '%') && (TempBuff[1] == 'c' || TempBuff[1] == 'C')) + else if (Type == SCANF_TYPE_CHAR) { - typeLen = sizeof(CHAR); + // Check whether the format string contains number of characters + // that should be read from the input string. + // Note: ‘\0’ does not get appended in the “%c” case. + typeLen = (Width > 0) ? Width : 1; } if (typeLen > 0) diff --git a/src/pal/tests/palsuite/paltestlist.txt b/src/pal/tests/palsuite/paltestlist.txt index a21e756..fa3183e 100644 --- a/src/pal/tests/palsuite/paltestlist.txt +++ b/src/pal/tests/palsuite/paltestlist.txt @@ -151,6 +151,8 @@ c_runtime/sprintf/test8/paltest_sprintf_test8 c_runtime/sprintf/test9/paltest_sprintf_test9 c_runtime/sqrt/test1/paltest_sqrt_test1 c_runtime/sscanf/test1/paltest_sscanf_test1 +c_runtime/sscanf/test10/paltest_sscanf_test10 +c_runtime/sscanf/test11/paltest_sscanf_test11 c_runtime/sscanf/test12/paltest_sscanf_test12 c_runtime/sscanf/test13/paltest_sscanf_test13 c_runtime/sscanf/test14/paltest_sscanf_test14 @@ -158,11 +160,13 @@ c_runtime/sscanf/test15/paltest_sscanf_test15 c_runtime/sscanf/test16/paltest_sscanf_test16 c_runtime/sscanf/test17/paltest_sscanf_test17 c_runtime/sscanf/test2/paltest_sscanf_test2 +c_runtime/sscanf/test3/paltest_sscanf_test3 c_runtime/sscanf/test4/paltest_sscanf_test4 c_runtime/sscanf/test5/paltest_sscanf_test5 c_runtime/sscanf/test6/paltest_sscanf_test6 c_runtime/sscanf/test7/paltest_sscanf_test7 c_runtime/sscanf/test8/paltest_sscanf_test8 +c_runtime/sscanf/test9/paltest_sscanf_test9 c_runtime/strcat/test1/paltest_strcat_test1 c_runtime/strchr/test1/paltest_strchr_test1 c_runtime/strcmp/test1/paltest_strcmp_test1 diff --git a/src/pal/tests/palsuite/paltestlist_to_be_reviewed.txt b/src/pal/tests/palsuite/paltestlist_to_be_reviewed.txt index 6e1b55f..0f135a0 100644 --- a/src/pal/tests/palsuite/paltestlist_to_be_reviewed.txt +++ b/src/pal/tests/palsuite/paltestlist_to_be_reviewed.txt @@ -15,10 +15,6 @@ c_runtime/fwprintf/test19/paltest_fwprintf_test19 c_runtime/fwprintf/test2/paltest_fwprintf_test2 c_runtime/fwprintf/test7/paltest_fwprintf_test7 c_runtime/iswprint/test1/paltest_iswprint_test1 -c_runtime/sscanf/test10/paltest_sscanf_test10 -c_runtime/sscanf/test11/paltest_sscanf_test11 -c_runtime/sscanf/test3/paltest_sscanf_test3 -c_runtime/sscanf/test9/paltest_sscanf_test9 c_runtime/swprintf/test2/paltest_swprintf_test2 c_runtime/swprintf/test7/paltest_swprintf_test7 c_runtime/ungetc/test2/paltest_ungetc_test2 -- 2.7.4