Guard against undefined shift.

xfa/fde/xml/fde_xml_imp.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "xfa/fde/xml/fde_xml_imp.h"
 
 #include <algorithm>
 
@@ skipping 1839 matching lines @@
   if (m_iEntityStart > -1 && ch == L';') {
     CFX_WideString csEntity;
     m_BlockBuffer.GetTextData(csEntity, m_iEntityStart + 1,
                               (m_iDataLength - 1) - m_iEntityStart - 1);
     int32_t iLen = csEntity.GetLength();
     if (iLen > 0) {
       if (csEntity[0] == L'#') {
         ch = 0;
         FX_WCHAR w;
         if (iLen > 1 && csEntity[1] == L'x') {
-          for (int32_t i = 2; i < iLen; i++) {
-            w = csEntity[i];
-            if (w >= L'0' && w <= L'9') {
-              ch = (ch << 4) + w - L'0';
-            } else if (w >= L'A' && w <= L'F') {
-              ch = (ch << 4) + w - 55;
-            } else if (w >= L'a' && w <= L'f') {
-              ch = (ch << 4) + w - 87;
-            } else {
-              break;
+          int32_t i = 2;
+          while (i < iLen && csEntity[i] == '0')
+            i++;
+          if (iLen - i <= 4) {
+            for (; i < iLen; i++) {
+              w = csEntity[i];
+              if (w >= L'0' && w <= L'9') {
+                ch = (ch << 4) + w - L'0';
+              } else if (w >= L'A' && w <= L'F') {
+                ch = (ch << 4) + w - 55;
+              } else if (w >= L'a' && w <= L'f') {
+                ch = (ch << 4) + w - 87;
+              } else {
+                break;
+              }
             }
+          } else {
+            ch = ' ';
           }
         } else {
           for (int32_t i = 1; i < iLen; i++) {
             w = csEntity[i];
-            if (w < L'0' || w > L'9') {
+            if (w < L'0' || w > L'9')
+              break;
+            ch = ch * 10 + w - L'0';
+
+            if (ch < 0) {

[Wei Li, 2016/08/10 17:28:30]

Could you also use length based checking here? If so, the test result would be
same across platforms.

[dsinclair, 2016/08/10 17:31:04]

I don't think so. This check doesn't do the shifts, but *10 each time. We could
do an initial check for length, but that wouldn't help for the case where the
value is INT_MAX + 1 that we're parsing. The length would be correct, but we're
going to overflow anyway.

[Wei Li, 2016/08/10 20:18:48]

Sorry for the back and forth. I am trying to find a solution consistent across
platforms. Maybe the following one works and simpler?

use unsigned value for calculation for both hex and decimal cases,
and check whether this value falls in [0, 10FFFF]
(https://www.w3.org/TR/xml/#wf-Legalchar then points to
https://www.w3.org/TR/xml/#NT-Char). 

WDYT?
 

[dsinclair, 2016/08/10 20:58:08]

No need to apologize, code working same on all platforms is definitely better.
How does this look?

+              ch = ' ';
               break;
             }
-            ch = ch * 10 + w - L'0';
           }
         }
         if (ch != 0) {
           m_BlockBuffer.SetTextChar(m_iEntityStart, ch);
           m_iEntityStart++;
         }
       } else {
         if (csEntity.Compare(L"amp") == 0) {
           m_BlockBuffer.SetTextChar(m_iEntityStart, L'&');
           m_iEntityStart++;
@@ skipping 15 matching lines @@
     m_BlockBuffer.DeleteTextChars(m_iDataLength - m_iEntityStart, FALSE);
     m_pCurrentBlock = m_BlockBuffer.GetAvailableBlock(m_iIndexInBlock);
     m_iEntityStart = -1;
   } else {
     if (m_iEntityStart < 0 && ch == L'&') {
       m_iEntityStart = m_iDataLength - 1;
     }
   }
   m_pStart++;
 }