Fix sscanf implementation in PAL

Implementation of sscanf was incorrectly handling cases where the input format was in the form of \93%2c\94, \93%2s\94, or \93%[\93 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.

diff --git a/src/pal/src/cruntime/printf.cpp b/src/pal/src/cruntime/printf.cpp
index bd22af0..aed5904 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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