[Libreoffice-commits] core.git: basic/qa basic/source include/rtl sal/rtl

[Mike Kaganski]

 basic/qa/basic_coverage/test_string_overflow_safe.vb |   22 +++++++++++++++++++
 basic/source/sbx/sbxvalue.cxx                        |    9 ++++++-
 include/rtl/ustring.hxx                              |   10 +++++++-
 sal/rtl/strtmpl.cxx                                  |    8 ++++--
 4 files changed, 43 insertions(+), 6 deletions(-)

New commits:
commit ef117cad3a13fda0932bd3da6c032f3499eb9069
Author: Mike Kaganski <mike.kaganski at collabora.com>
Date:   Tue May 23 23:57:50 2017 +0300

    tdf#108039: check for nullptr in rtl_uString and OUString
    
    rtl_[u]String_newConcat now checks allocation result to return
    early and avoid SIGSEGV. Other functions are not modified, to
    keep old behavior relying on allocation success and crashing
    early on OOM to avoid added overhead in performance-critical
    places.
    
    OUString operator+= now checks rtl_uString_newConcat result and
    throws std::bad_alloc on failure, to specifically address BASIC
    problem. It keeps strong exception guarantee of leaving this'
    state unaltered.
    
    Concatenation in BASIC now checks for bad string allocation
    (previously SIGSEGV was generated).
    
    Unit test included.
    
    Change-Id: I1513311d3d58eac43b2d2ec9a230e22dff0b4245
    Reviewed-on: https://gerrit.libreoffice.org/37965
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
    Tested-by: Jenkins <ci at libreoffice.org>

diff --git a/basic/qa/basic_coverage/test_string_overflow_safe.vb b/basic/qa/basic_coverage/test_string_overflow_safe.vb
new file mode 100644
index 000000000000..148cc910c7f5
--- /dev/null
+++ b/basic/qa/basic_coverage/test_string_overflow_safe.vb
@@ -0,0 +1,22 @@
+Option Explicit
+
+Function doUnitTest As Integer
+    ' Trying to create too long string should generate proper BASIC overflow error.
+    ' Longest possible string is 2147483638 wchar_t (2G - 10).
+    ' This tries to create string with 2G wchar_t. If it does not overflow, test fails.
+    ' If overflow is not safe, it segfaults.
+    On Error GoTo errorHandler
+    Dim s As String, i As Integer
+    s = "0"
+    For i=1 To 31
+    s = s & s
+    Next i
+    doUnitTest = 0
+    Exit Function
+errorHandler:
+    If ( Err <> 6 ) Then
+        doUnitTest = 0
+    Else
+        doUnitTest = 1
+    Endif
+End Function
diff --git a/basic/source/sbx/sbxvalue.cxx b/basic/source/sbx/sbxvalue.cxx
index bca0f80f8d2f..7ff177d8a736 100644
--- a/basic/source/sbx/sbxvalue.cxx
+++ b/basic/source/sbx/sbxvalue.cxx
@@ -875,14 +875,19 @@ bool SbxValue::Compute( SbxOperator eOp, const SbxValue& rOp )
                 // #30576: To begin with test, if the conversion worked
                 if( aL.pOUString != nullptr && aR.pOUString != nullptr )
                 {
-                    *aL.pOUString += *aR.pOUString;
+                    // tdf#108039: catch possible bad_alloc
+                    try {
+                        *aL.pOUString += *aR.pOUString;
+                    }
+                    catch (const std::bad_alloc&) {
+                        SetError(ERRCODE_SBX_OVERFLOW);
+                    }
                 }
                 // Not even Left OK?
                 else if( aL.pOUString == nullptr )
                 {
                     aL.pOUString = new OUString();
                 }
-                Put( aL );
             }
             else
                 SetError( ERRCODE_SBX_CONVERSION );
diff --git a/include/rtl/ustring.hxx b/include/rtl/ustring.hxx
index 8b1cd3a1404f..e4dfe8d8346d 100644
--- a/include/rtl/ustring.hxx
+++ b/include/rtl/ustring.hxx
@@ -519,13 +519,21 @@ public:
       Append a string to this string.
 
       @param    str         a OUString.
+
+      @exception std::bad_alloc is thrown if an out-of-memory condition occurs
     */
     OUString & operator+=( const OUString & str )
 #if defined LIBO_INTERNAL_ONLY && HAVE_CXX11_REF_QUALIFIER
         &
 #endif
     {
-        rtl_uString_newConcat( &pData, pData, str.pData );
+        rtl_uString* pNewData = NULL;
+        rtl_uString_newConcat( &pNewData, pData, str.pData );
+        if (pNewData == NULL) {
+            throw std::bad_alloc();
+        }
+        rtl_uString_assign(&pData, pNewData);
+        rtl_uString_release(pNewData);
         return *this;
     }
 #if defined LIBO_INTERNAL_ONLY && HAVE_CXX11_REF_QUALIFIER
diff --git a/sal/rtl/strtmpl.cxx b/sal/rtl/strtmpl.cxx
index 6748209fb50f..3ab7698877ad 100644
--- a/sal/rtl/strtmpl.cxx
+++ b/sal/rtl/strtmpl.cxx
@@ -1507,11 +1507,13 @@ void SAL_CALL IMPL_RTL_STRINGNAME( newConcat )( IMPL_RTL_STRINGDATA** ppThis,
     {
         IMPL_RTL_STRINGDATA* pTempStr = IMPL_RTL_STRINGNAME( ImplAlloc )( pLeft->length + pRight->length );
         OSL_ASSERT(pTempStr != nullptr);
-        rtl_str_ImplCopy( pTempStr->buffer, pLeft->buffer, pLeft->length );
-        rtl_str_ImplCopy( pTempStr->buffer+pLeft->length, pRight->buffer, pRight->length );
         *ppThis = pTempStr;
+        if (*ppThis != nullptr) {
+            rtl_str_ImplCopy( pTempStr->buffer, pLeft->buffer, pLeft->length );
+            rtl_str_ImplCopy( pTempStr->buffer+pLeft->length, pRight->buffer, pRight->length );
 
-        RTL_LOG_STRING_NEW( *ppThis );
+            RTL_LOG_STRING_NEW( *ppThis );
+        }
     }
 
     /* must be done last, if left or right == *ppThis */