[Libreoffice-commits] core.git: xmlhelp/source

[Stephan Bergmann (via logerrit)]

 xmlhelp/source/cxxhelp/provider/db.cxx |   37 ++++++++++++++++++++++++---------
 xmlhelp/source/cxxhelp/provider/db.hxx |    5 +---
 2 files changed, 29 insertions(+), 13 deletions(-)

New commits:
commit daffec714ef28f52ae2d70e5553500046b8b26c2
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Fri Jul 23 12:20:05 2021 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Fri Jul 23 14:03:11 2021 +0200

    Avoid strtol on non-NUL-terminated input
    
    ...as could be observed with ASan and ASAN_OPTIONS=strict_string_checks=1 during
    e.g. UITest_conditional_format.
    
    strtol supports leading spaces, sign, and "0x", but none of that appears to be
    relevant here, so readInt32 doesn't support that.
    
    Hdf::m_pItData was redundant and always pointed at m_aItData.data(), and has
    been elided to make it more obvious that the places that used it, which now also
    need to use m_aItData.size(), actually use m_aItData.
    
    Change-Id: I3d4f625cd5836438271d7c1a8d2ae06f0a45a37c
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/119403
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/xmlhelp/source/cxxhelp/provider/db.cxx b/xmlhelp/source/cxxhelp/provider/db.cxx
index 1e376e83e32d..bf3c269b99ca 100644
--- a/xmlhelp/source/cxxhelp/provider/db.cxx
+++ b/xmlhelp/source/cxxhelp/provider/db.cxx
@@ -21,12 +21,31 @@
 #include "db.hxx"
 
 #include <cstring>
+#include <utility>
 
 #include <com/sun/star/io/XSeekable.hpp>
+#include <tools/inetmime.hxx>
 
 using namespace com::sun::star::uno;
 using namespace com::sun::star::io;
 
+namespace {
+
+//TODO: Replace with C++17 std::from_chars once available:
+std::pair<sal_Int32, char const *> readInt32(char const * begin, char const * end) {
+    sal_Int32 n = 0;
+    for (; begin != end; ++begin) {
+        auto const w = INetMIME::getHexWeight(static_cast<unsigned char>(*begin));
+        if (w == -1) {
+            break;
+        }
+        n = 16 * n + w;
+    }
+    return {n, begin};
+}
+
+}
+
 namespace helpdatafileproxy {
 
 void HDFData::copyToBuffer( const char* pSrcData, int nSize )
@@ -40,14 +59,13 @@ void HDFData::copyToBuffer( const char* pSrcData, int nSize )
 
 // Hdf
 
-bool Hdf::implReadLenAndData( const char* pData, int& riPos, HDFData& rValue )
+bool Hdf::implReadLenAndData( const char* pData, char const * end, int& riPos, HDFData& rValue )
 {
     bool bSuccess = false;
 
     // Read key len
     const char* pStartPtr = pData + riPos;
-    char* pEndPtr;
-    sal_Int32 nKeyLen = strtol( pStartPtr, &pEndPtr, 16 );
+    auto [nKeyLen, pEndPtr] = readInt32(pStartPtr, end);
     if( pEndPtr == pStartPtr )
         return bSuccess;
     riPos += (pEndPtr - pStartPtr) + 1;
@@ -85,19 +103,19 @@ void Hdf::createHashMap( bool bOptimizeForPerformance )
     sal_Int32 nRead = xIn->readBytes( aData, nSize );
 
     const char* pData = reinterpret_cast<const char*>(aData.getConstArray());
+    auto const end = pData + nRead;
     int iPos = 0;
     while( iPos < nRead )
     {
         HDFData aDBKey;
-        if( !implReadLenAndData( pData, iPos, aDBKey ) )
+        if( !implReadLenAndData( pData, end, iPos, aDBKey ) )
             break;
 
         OString aOKeyStr = aDBKey.getData();
 
         // Read val len
         const char* pStartPtr = pData + iPos;
-        char* pEndPtr;
-        sal_Int32 nValLen = strtol( pStartPtr, &pEndPtr, 16 );
+        auto [nValLen, pEndPtr] = readInt32(pStartPtr, end);
         if( pEndPtr == pStartPtr )
             break;
 
@@ -211,7 +229,6 @@ bool Hdf::startIteration()
         if( m_nItRead == nSize )
         {
             bSuccess = true;
-            m_pItData = reinterpret_cast<const char*>(m_aItData.getConstArray());
             m_iItPos = 0;
         }
         else
@@ -229,9 +246,10 @@ bool Hdf::getNextKeyAndValue( HDFData& rKey, HDFData& rValue )
 
     if( m_iItPos < m_nItRead )
     {
-        if( implReadLenAndData( m_pItData, m_iItPos, rKey ) )
+        auto const p = reinterpret_cast<const char*>(m_aItData.getConstArray());
+        if( implReadLenAndData( p, p + m_aItData.size(), m_iItPos, rKey ) )
         {
-            if( implReadLenAndData( m_pItData, m_iItPos, rValue ) )
+            if( implReadLenAndData( p, p + m_aItData.size(), m_iItPos, rValue ) )
                 bSuccess = true;
         }
     }
@@ -242,7 +260,6 @@ bool Hdf::getNextKeyAndValue( HDFData& rKey, HDFData& rValue )
 void Hdf::stopIteration()
 {
     m_aItData = Sequence<sal_Int8>();
-    m_pItData = nullptr;
     m_nItRead = -1;
     m_iItPos = -1;
 }
diff --git a/xmlhelp/source/cxxhelp/provider/db.hxx b/xmlhelp/source/cxxhelp/provider/db.hxx
index 11ec0a826916..aa02903bd16d 100644
--- a/xmlhelp/source/cxxhelp/provider/db.hxx
+++ b/xmlhelp/source/cxxhelp/provider/db.hxx
@@ -58,11 +58,11 @@ namespace helpdatafileproxy {
 
         css::uno::Sequence< sal_Int8 >
                             m_aItData;
-        const char*         m_pItData;
         int                 m_nItRead;
         int                 m_iItPos;
 
-        static bool implReadLenAndData( const char* pData, int& riPos, HDFData& rValue );
+        static bool implReadLenAndData(
+            const char* pData, char const * end, int& riPos, HDFData& rValue );
 
     public:
         //HDFHelp must get a fileURL which can then directly be used by simple file access.
@@ -72,7 +72,6 @@ namespace helpdatafileproxy {
             css::uno::Reference< css::ucb::XSimpleFileAccess3 > const & xSFA )
                 : m_aFileURL( rFileURL )
                 , m_xSFA( xSFA )
-                , m_pItData( nullptr )
                 , m_nItRead( -1 )
                 , m_iItPos( -1 )
         {