Fix bounds checking in CJS_PublicMethods::MakeRegularDate().
authorTom Sepez <tsepez@chromium.org>
Fri, 18 Jul 2014 21:42:12 +0000 (14:42 -0700)
committerTom Sepez <tsepez@chromium.org>
Fri, 18 Jul 2014 21:42:12 +0000 (14:42 -0700)
The function is looking ahead N characters at both its "format" and "value"
strings without validating that accesses are in bounds.  Add those validations.

There are also duplicate checks in the else-branches which re-test the inverse
of the if-branch.  These are removed for simplicity.

I also tidied some stray whitespace in the function while I was at it.

BUG=393831
R=jun_fang@foxitsoftware.com

Review URL: https://codereview.chromium.org/395303004

AUTHORS
fpdfsdk/src/javascript/PublicMethods.cpp

diff --git a/AUTHORS b/AUTHORS
index f12a529..7d83424 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -24,6 +24,7 @@ Nico Weber <thakis@chromium.org>
 Raymes Khoury <raymes@chromium.org>
 Reid Kleckner <rnk@chromium.org>
 Robert Sesek <rsesek@chromium.org>
+Thomas Sepez <tsepez@chromium.org>
 
 Foxit Software Inc <*@foxitsoftware.com>
 Google Inc. <*@google.com>
index bd4acae..5522868 100644 (file)
@@ -624,7 +624,7 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
        FX_BOOL bPm = FALSE;
        FX_BOOL bExit = FALSE;
        bWrongFormat = FALSE;
-       
+
        int i=0;
        int j=0;
 
@@ -632,7 +632,7 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
        {
                if (bExit) break;
 
-               FX_WCHAR c = format.GetAt(i);           
+               FX_WCHAR c = format.GetAt(i);
                switch (c)
                {
                        case ':':
@@ -643,7 +643,7 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
                                i++;
                                j++;
                                break;
-                               
+
                        case 'y':
                        case 'm':
                        case 'd':
@@ -655,8 +655,9 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
                                {
                                        int oldj = j;
                                        int nSkip = 0;
+                                       int remaining = format.GetLength() - i - 1;
 
-                                       if (format.GetAt(i+1) != c)
+                                       if (remaining == 0 || format.GetAt(i+1) != c)
                                        {
                                                switch (c)
                                                {
@@ -695,13 +696,13 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
                                                                j += nSkip;
                                                                break;
                                                        case 't':
-                                                               bPm = value.GetAt(i) == 'p';
+                                                               bPm = (j < value.GetLength() && value.GetAt(j) == 'p');
                                                                i++;
                                                                j++;
                                                                break;
-                                               }                                       
+                                               }
                                        }
-                                       else if (format.GetAt(i+1) == c && format.GetAt(i+2) != c)
+                                       else if (remaining == 1 || format.GetAt(i+2) != c)
                                        {
                                                switch (c)
                                                {
@@ -741,13 +742,13 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
                                                                j += nSkip;
                                                                break;
                                                        case 't':
-                                                               bPm = (value.GetAt(j) == 'p' && value.GetAt(j+1) == 'm');
+                                                               bPm = (j + 1 < value.GetLength() && value.GetAt(j) == 'p' && value.GetAt(j+1) == 'm');
                                                                i += 2;
                                                                j += 2;
                                                                break;
                                                }
                                        }
-                                       else if (format.GetAt(i+1) == c && format.GetAt(i+2) == c && format.GetAt(i+3) != c)
+                                       else if (remaining == 2 || format.GetAt(i+3) != c)
                                        {
                                                switch (c)
                                                {
@@ -766,7 +767,7 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
                                                                                        break;
                                                                                }
                                                                        }
-                                                                       
+
                                                                        if (!bFind)
                                                                        {
                                                                                nMonth = ParseStringInteger(value, j, nSkip, 3);
@@ -783,7 +784,7 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
                                                                break;
                                                }
                                        }
-                                       else if (format.GetAt(i+1) == c && format.GetAt(i+2) == c && format.GetAt(i+3) == c && format.GetAt(i+4) != c)
+                                       else if (remaining == 3 || format.GetAt(i+4) != c)
                                        {
                                                switch (c)
                                                {
@@ -815,7 +816,7 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
                                                                                        break;
                                                                                }
                                                                        }
-                                                                       
+
                                                                        if (!bFind)
                                                                        {
                                                                                nMonth = ParseStringInteger(value, j, nSkip, 4);
@@ -828,11 +829,11 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
                                                                i += 4;
                                                                j += 4;
                                                                break;
-                                               }                                       
+                                               }
                                        }
                                        else
                                        {
-                                               if (format.GetAt(i) != value.GetAt(j))
+                                               if (j >= value.GetLength() || format.GetAt(i) != value.GetAt(j))
                                                {
                                                        bWrongFormat = TRUE;
                                                        bExit = TRUE;
@@ -840,7 +841,7 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
                                                i++;
                                                j++;
                                        }
-                                       
+
                                        if (oldj == j)
                                        {
                                                bWrongFormat = TRUE;
@@ -848,7 +849,7 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
                                        }
                                }
 
-                               break;                  
+                               break;
                        default:
                                if (value.GetLength() <= j)
                                {
@@ -863,7 +864,7 @@ double CJS_PublicMethods::MakeRegularDate(const CFX_WideString & value, const CF
                                i++;
                                j++;
                                break;
-               }               
+               }
        }
 
        if (bPm) nHour += 12;