[Libreoffice-commits] online.git: 2 commits - common/Protocol.cpp common/Session.cpp common/Util.cpp common/Util.hpp configure.ac fuzzer/ClientSession.cpp fuzzer/data Makefile.am wsd/LOOLWSD.cpp wsd/LOOLWSD.hpp

Miklos Vajna (via logerrit) logerrit at kemper.freedesktop.org
Sat Feb 22 11:18:31 UTC 2020


 Makefile.am                                                |   24 +++++++
 common/Protocol.cpp                                        |   11 ++-
 common/Session.cpp                                         |    2 
 common/Util.cpp                                            |    9 ++
 common/Util.hpp                                            |    6 +
 configure.ac                                               |   19 ++++++
 fuzzer/ClientSession.cpp                                   |   40 +++++++++++++
 fuzzer/data/clientvisiblearea                              |    1 
 fuzzer/data/crash-060b81ab7163c0546b2c11459f19719af22d7390 |binary
 fuzzer/data/load                                           |    1 
 fuzzer/data/loolclient                                     |    1 
 fuzzer/data/textinput                                      |    2 
 fuzzer/data/tileprocessed                                  |    1 
 wsd/LOOLWSD.cpp                                            |    3 
 wsd/LOOLWSD.hpp                                            |    5 +
 15 files changed, 119 insertions(+), 6 deletions(-)

New commits:
commit 57a35bb96c18ae552ce9165eb25120222a58bfee
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Fri Feb 21 15:52:20 2020 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Sat Feb 22 12:18:22 2020 +0100

    Add an initial libfuzzer based fuzzer
    
    - target ClientSession::_handleInput(), since crashing there would bring
      down the whole loolwsd (not just a kit process), and it deals with
      input from untrusted users (browsers)
    
    - add a --enable-fuzzers configure switch to build with
      -fsanitize=fuzzer (compared to normal sanitizers build, this is the only
      special flag needed)
    
    - configuring other sanitizers is not done automatically, either use
      --with-sanitizer=... or the environment variables from LODE's sanitizer
      config
    
    - run the actual fuzzer like this:
    
      ./clientsession_fuzzer -max_len=16384 fuzzer/data/
    
    - note that at least openSUSE Leap 15.1 sadly ships with a clang with
      libfuzzer static libs removed from the package, so you need a
      self-built clang to run the fuzzer (either manual build or one from
      LODE)
    
    - <https://chromium.googlesource.com/chromium/src/testing/libfuzzer/+/refs/heads/master/efficient_fuzzing.md#execution-speed>
      suggests that "You should aim for at least 1,000 exec/s from your fuzz
      target locally" (i.e. one run should not take more than 1 ms), so try
      this minimal approach first. The alternative would be to start from the
      existing loolwsd_fuzzer binary, then step by step cut it down to not
      fork(), not do any network traffic, etc -- till it's fast enough that
      the fuzzer can find interesting input
    
    - the various configurations start to be really complex (the matrix is
      just very large), so try to use Util::isFuzzing() for fuzzer-specific
      changes (this is what core.git does as well), and only resort to ifdefs
      for the Util::isFuzzing() itself
    
    Change-Id: I72dc1193b34c93eacb5d8e39cef42387d42bd72f
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89226
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/Makefile.am b/Makefile.am
index c6d417185..1ee125b1f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -19,9 +19,14 @@ SUBDIRS = . test loleaflet cypress_test
 export ENABLE_DEBUG
 
 bin_PROGRAMS = \
-	loolwsd loolforkit \
+	loolforkit \
 	loolconvert loolconfig
 
+if ENABLE_LIBFUZZER
+else
+bin_PROGRAMS += loolwsd
+endif
+
 dist_bin_SCRIPTS = loolwsd-systemplate-setup
 
 man_MANS = man/loolwsd.1 \
@@ -119,12 +124,17 @@ loolwsd_SOURCES = $(loolwsd_sources) \
 noinst_PROGRAMS = clientnb \
                   connect \
                   lokitclient \
-                  loolwsd_fuzzer \
                   loolmap \
                   loolstress \
                   loolmount \
                   loolsocketdump
 
+if ENABLE_LIBFUZZER
+noinst_PROGRAMS += clientsession_fuzzer
+else
+noinst_PROGRAMS += loolwsd_fuzzer
+endif
+
 connect_SOURCES = tools/Connect.cpp \
                   common/Log.cpp \
                   common/Protocol.cpp \
@@ -148,6 +158,16 @@ loolwsd_fuzzer_SOURCES = $(loolwsd_sources) \
                          $(shared_sources) \
                          kit/DummyLibreOfficeKit.cpp
 
+clientsession_fuzzer_CPPFLAGS = \
+				-DKIT_IN_PROCESS=1 \
+				$(AM_CPPFLAGS)
+clientsession_fuzzer_SOURCES = \
+			       $(loolwsd_sources) \
+			       $(loolforkit_sources) \
+			       $(shared_sources) \
+			       fuzzer/ClientSession.cpp
+clientsession_fuzzer_LDFLAGS = -fsanitize=fuzzer $(AM_LDFLAGS)
+
 clientnb_SOURCES = net/clientnb.cpp \
                    common/Log.cpp \
                    common/Util.cpp
diff --git a/common/Session.cpp b/common/Session.cpp
index 37f3a5952..2a51b0e89 100644
--- a/common/Session.cpp
+++ b/common/Session.cpp
@@ -219,7 +219,7 @@ void Session::handleMessage(bool /*fin*/, WSOpCode /*code*/, std::vector<char> &
     try
     {
         std::unique_ptr< std::vector<char> > replace;
-        if (UnitBase::get().filterSessionInput(this, &data[0], data.size(), replace))
+        if (!Util::isFuzzing() && UnitBase::get().filterSessionInput(this, &data[0], data.size(), replace))
         {
             if (!replace || replace->empty())
                 _handleInput(replace->data(), replace->size());
diff --git a/common/Util.cpp b/common/Util.cpp
index 0b3e8cbae..f9df08f2f 100644
--- a/common/Util.cpp
+++ b/common/Util.cpp
@@ -913,6 +913,15 @@ namespace Util
                 std::chrono::system_clock::now() + (time - now)));
         return std::ctime(&t);
     }
+
+    bool isFuzzing()
+    {
+#if LIBFUZZER
+        return true;
+#else
+        return false;
+#endif
+    }
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/common/Util.hpp b/common/Util.hpp
index 5dc39f942..991b548ef 100644
--- a/common/Util.hpp
+++ b/common/Util.hpp
@@ -964,6 +964,12 @@ int main(int argc, char**argv)
 
         std::function<void()> m_func;
     };
+
+    /**
+     * Avoid using the configuration layer and rely on defaults which is only useful for special
+     * test tool targets (typically fuzzing) where start-up speed is critical.
+     */
+    bool isFuzzing();
 } // end namespace Util
 
 #endif
diff --git a/configure.ac b/configure.ac
index 9ae4584d7..398d303e3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -122,6 +122,11 @@ AC_ARG_ENABLE([gtkapp],
                               to work similarly to the iOS app, from the JavaScript and the pseudo WebSocket
                               message plumbing point of view. See gtk/README.]))
 
+AC_ARG_ENABLE(fuzzers,
+    AS_HELP_STRING([--enable-fuzzers],
+        [Enables building libfuzzer targets for fuzz testing.])
+)
+
 AC_ARG_ENABLE([androidapp],
               AS_HELP_STRING([--enable-androidapp],
                              [Use in a tree where the only purpose is to build the Android app that is supposed
@@ -585,6 +590,20 @@ else
     AC_MSG_RESULT([no])
 fi
 
+AC_MSG_CHECKING([whether to build fuzzers])
+if test "$enable_fuzzers" = "yes"; then
+    AC_MSG_RESULT([yes])
+    LIBFUZZER_FLAGS="-fsanitize=fuzzer-no-link"
+    CXXFLAGS="$CXXFLAGS $LIBFUZZER_FLAGS"
+    CFLAGS="$CFLAGS $LIBFUZZER_FLAGS"
+    LIBFUZZER=1
+else
+    AC_MSG_RESULT([no])
+    LIBFUZZER=0
+fi
+AC_DEFINE_UNQUOTED([LIBFUZZER],[$LIBFUZZER],[Define to 1 if this is a libfuzzer build.])
+AM_CONDITIONAL([ENABLE_LIBFUZZER], [test "$LIBFUZZER" = "1"])
+
 # check for C++11 support
 HAVE_CXX11=
 AC_MSG_CHECKING([whether $CXX supports C++17, C++14 or C++11])
diff --git a/fuzzer/ClientSession.cpp b/fuzzer/ClientSession.cpp
new file mode 100644
index 000000000..53872d5ca
--- /dev/null
+++ b/fuzzer/ClientSession.cpp
@@ -0,0 +1,40 @@
+#include <iostream>
+
+#include "ClientSession.hpp"
+
+bool DoInitialization()
+{
+    LOOLWSD::ChildRoot = "/fuzz/child-root";
+    return true;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
+{
+    static bool initialized = DoInitialization();
+    (void)initialized;
+
+    std::string uri;
+    Poco::URI uriPublic;
+    std::string docKey = "/fuzz/fuzz.odt";
+    auto docBroker = std::make_shared<DocumentBroker>(uri, uriPublic, docKey);
+
+    std::string id;
+    bool isReadOnly = false;
+    const std::string hostNoTrust;
+    auto session
+        = std::make_shared<ClientSession>(id, docBroker, uriPublic, isReadOnly, hostNoTrust);
+
+    bool fin = false;
+    WSOpCode code = WSOpCode::Text;
+    std::string input(reinterpret_cast<const char*>(data), size);
+    std::stringstream ss(input);
+    std::string line;
+    while (std::getline(ss, line, '\n'))
+    {
+        std::vector<char> lineVector(line.data(), line.data() + line.size());
+        session->handleMessage(fin, code, lineVector);
+    }
+    return 0;
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/fuzzer/data/clientvisiblearea b/fuzzer/data/clientvisiblearea
new file mode 100644
index 000000000..532ddf12d
--- /dev/null
+++ b/fuzzer/data/clientvisiblearea
@@ -0,0 +1 @@
+clientvisiblearea x=-7995 y=0 width=22530 height=11730
diff --git a/fuzzer/data/load b/fuzzer/data/load
new file mode 100644
index 000000000..42cb184b4
--- /dev/null
+++ b/fuzzer/data/load
@@ -0,0 +1 @@
+load url=file%3A%2F%2F%2Ffuzz%2Ffuzz.odt part=0 lang=en-US
diff --git a/fuzzer/data/loolclient b/fuzzer/data/loolclient
new file mode 100644
index 000000000..d6dde14df
--- /dev/null
+++ b/fuzzer/data/loolclient
@@ -0,0 +1 @@
+loolclient 0.1
diff --git a/fuzzer/data/textinput b/fuzzer/data/textinput
new file mode 100644
index 000000000..3221db0ba
--- /dev/null
+++ b/fuzzer/data/textinput
@@ -0,0 +1,2 @@
+textinput id=0 type=input text=a
+textinput id=0 type=end text=a
diff --git a/fuzzer/data/tileprocessed b/fuzzer/data/tileprocessed
new file mode 100644
index 000000000..ad6960b0e
--- /dev/null
+++ b/fuzzer/data/tileprocessed
@@ -0,0 +1 @@
+tileprocessed tile=0:0:0:3840:3840:0
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 3527f8770..678b4ddc6 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -3734,7 +3734,8 @@ void dump_state()
     LOG_TRC(msg);
 }
 
-#if !MOBILEAPP
+// Avoid this in the Util::isFuzzing() case because libfuzzer defines its own main().
+#if !MOBILEAPP && !LIBFUZZER
 
 POCO_SERVER_MAIN(LOOLWSD)
 
diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp
index 288ed6bb9..d14b77ae5 100644
--- a/wsd/LOOLWSD.hpp
+++ b/wsd/LOOLWSD.hpp
@@ -133,6 +133,11 @@ public:
     static
     T getConfigValue(const std::string& name, const T def)
     {
+        if (Util::isFuzzing())
+        {
+            return def;
+        }
+
         return getConfigValue(Application::instance().config(), name, def);
     }
 
commit 8d2a8da960828d16502927f80ad76fabf502df6d
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Fri Feb 21 15:36:28 2020 +0100
Commit:     Michael Meeks <michael.meeks at collabora.com>
CommitDate: Sat Feb 22 12:18:11 2020 +0100

    common: fix crash when the version string contains no dot character
    
    ==13901==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000904678 bp 0x7ffdb9e21580 sp 0x7ffdb9e21340 T0)
    ==13901==The signal is caused by a READ memory access.
    ==13901==Hint: address points to the zero page.
        #0 0x904677 in LOOLProtocol::tokenize[abi:cxx11](char const*, unsigned long, char) common/Protocol.hpp:113:40
        #1 0x898c52 in LOOLProtocol::tokenize(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, char) common/Protocol.hpp:141:16
        #2 0x18dc2d9 in LOOLProtocol::ParseVersion(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) common/Protocol.cpp:35:51
        #3 0x1148824 in ClientSession::_handleInput(char const*, int) wsd/ClientSession.cpp:358:64
        #4 0x18efcb8 in Session::handleMessage(bool, WSOpCode, std::vector<char, std::allocator<char> >&) common/Session.cpp:232:13
    
    Next commit will add the actual simple fuzzer that found this.
    
    Change-Id: I8623b4451a57390f6f84c11084c5a1120a11fcc5
    Reviewed-on: https://gerrit.libreoffice.org/c/online/+/89225
    Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/common/Protocol.cpp b/common/Protocol.cpp
index e915f8085..4d8f51c50 100644
--- a/common/Protocol.cpp
+++ b/common/Protocol.cpp
@@ -32,8 +32,15 @@ namespace LOOLProtocol
         {
             major = std::stoi(firstTokens[0]);
 
-            std::vector<std::string> secondTokens(tokenize(firstTokens[1], '-'));
-            minor = std::stoi(secondTokens[0]);
+            std::vector<std::string> secondTokens;
+            if (firstTokens.size() > 1)
+            {
+                secondTokens = tokenize(firstTokens[1], '-');
+            }
+            if (secondTokens.size() > 0)
+            {
+                minor = std::stoi(secondTokens[0]);
+            }
 
             if (secondTokens.size() > 1)
                 patch = secondTokens[1];
diff --git a/fuzzer/data/crash-060b81ab7163c0546b2c11459f19719af22d7390 b/fuzzer/data/crash-060b81ab7163c0546b2c11459f19719af22d7390
new file mode 100644
index 000000000..7f94fa866
Binary files /dev/null and b/fuzzer/data/crash-060b81ab7163c0546b2c11459f19719af22d7390 differ


More information about the Libreoffice-commits mailing list