Simplify getToken

It should not attempt to dereference pointers when nIndex is negative.
Properly handle too large nIndex.
Also it is not necessary to parse the string when nToken is negative.

Related to commit be281db569bafaac379feb604c39e220f51b18c4

Author Rüdiger Timm <rt@openoffice.org>
Date   Mon Sep 20 07:43:20 2004 +0000

INTEGRATION: CWS ause011 (1.18.22); FILE MERGED
2004/08/18 11:47:54 sb 1.18.22.1: #i33153# Made getToken more robust.

Change-Id: I6fc77a5b70308ccca08cb2132bd78d024bd7e3e0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/131221
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

2 files changed, 49 insertions, 37 deletions

diff --git a/sal/qa/rtl/oustring/rtl_OUString2.cxx b/sal/qa/rtl/oustring/rtl_OUString2.cxx
index 0ce047e9fb32..5f68629a4c09 100644
--- a/sal/qa/rtl/oustring/rtl_OUString2.cxx
+++ b/sal/qa/rtl/oustring/rtl_OUString2.cxx
@@ -32,6 +32,7 @@
 
 #include <config_options.h>
 #include <o3tl/cppunittraitshelper.hxx>
+#include <o3tl/safeint.hxx>
 
 #include <stringhelper.hxx>
 #include <valueequal.hxx>
@@ -781,6 +782,26 @@ public:
             "token should be empty", ab.getToken(0, '-', n).isEmpty());
     }
 
+    void getToken_006()
+    {
+        OUString suTokenStr;
+        auto pTokenStr = suTokenStr.getStr();
+        sal_uInt64 n64 = reinterpret_cast<sal_uInt64>(pTokenStr) / sizeof(sal_Unicode);
+        // Point either to 0x0, or to some random address -4GiB away from this string
+        sal_Int32 n = n64 > o3tl::make_unsigned(SAL_MAX_INT32) ? -SAL_MAX_INT32
+                                                               : -static_cast<sal_Int32>(n64);
+        suTokenStr.getToken(0, ';', n);
+        // should not GPF with negative index
+    }
+
+    void getToken_007()
+    {
+        OUString suTokenStr("a;b");
+        sal_Int32 n = 5; // greater than string length
+        CPPUNIT_ASSERT_EQUAL(OUString(), suTokenStr.getToken(0, ';', n));
+        CPPUNIT_ASSERT_EQUAL(sal_Int32(-1), n);
+    }
+
     CPPUNIT_TEST_SUITE(getToken);
     CPPUNIT_TEST(getToken_000);
     CPPUNIT_TEST(getToken_001);
@@ -788,6 +809,8 @@ public:
     CPPUNIT_TEST(getToken_003);
     CPPUNIT_TEST(getToken_004);
     CPPUNIT_TEST(getToken_005);
+    CPPUNIT_TEST(getToken_006);
+    CPPUNIT_TEST(getToken_007);
     CPPUNIT_TEST_SUITE_END();
 }; // class getToken
 
diff --git a/sal/rtl/strtmpl.hxx b/sal/rtl/strtmpl.hxx
index 37f274a69cf2..0f0cc6755eb3 100644
--- a/sal/rtl/strtmpl.hxx
+++ b/sal/rtl/strtmpl.hxx
@@ -1256,54 +1256,43 @@ sal_Int32 getToken                                ( IMPL_RTL_STRINGDATA** ppThis
 {
     assert(ppThis);
     assert(pStr);
-    const auto*             pCharStr        = pStr->buffer;
-    sal_Int32               nLen            = pStr->length-nIndex;
-    sal_Int32               nTokCount       = 0;
 
     // Set ppThis to an empty string and return -1 if either nToken or nIndex is
     // negative:
-    if (nIndex < 0)
-        nToken = -1;
-
-    pCharStr += nIndex;
-    const auto* pOrgCharStr = pCharStr;
-    const auto* pCharStrStart = pCharStr;
-    while ( nLen > 0 )
+    if (nIndex >= 0 && nToken >= 0)
     {
-        if ( *pCharStr == cTok )
+        const auto* pOrgCharStr = pStr->buffer;
+        const auto* pCharStr = pOrgCharStr + nIndex;
+        sal_Int32 nLen = pStr->length - nIndex;
+        sal_Int32 nTokCount = 0;
+        const auto* pCharStrStart = pCharStr;
+        while (nLen > 0)
         {
-            nTokCount++;
-
-            if ( nTokCount == nToken )
-                pCharStrStart = pCharStr+1;
-            else
+            if (*pCharStr == cTok)
             {
-                if ( nTokCount > nToken )
+                nTokCount++;
+
+                if (nTokCount > nToken)
                     break;
+                if (nTokCount == nToken)
+                    pCharStrStart = pCharStr + 1;
             }
-        }
 
-        pCharStr++;
-        nLen--;
+            pCharStr++;
+            nLen--;
+        }
+        if (nTokCount >= nToken)
+        {
+            newFromStr_WithLength(ppThis, pCharStrStart, pCharStr - pCharStrStart);
+            if (nLen > 0)
+                return pCharStr - pOrgCharStr + 1;
+            else
+                return -1;
+        }
     }
 
-    if ( (nToken < 0) || (nTokCount < nToken) || (pCharStr == pCharStrStart) )
-    {
-        new_( ppThis );
-        if( (nToken < 0) || (nTokCount < nToken) )
-            return -1;
-        else if( nLen > 0 )
-            return nIndex+(pCharStr-pOrgCharStr)+1;
-        else return -1;
-    }
-    else
-    {
-        newFromStr_WithLength( ppThis, pCharStrStart, pCharStr-pCharStrStart );
-        if ( nLen )
-            return nIndex+(pCharStr-pOrgCharStr)+1;
-        else
-            return -1;
-    }
+    new_(ppThis);
+    return -1;
 }
 
 namespace detail