common/Authorization.cpp | 33 ++++++++++++++++++++++++++++++--- common/Util.cpp | 34 +--------------------------------- common/Util.hpp | 7 ------- 3 files changed, 31 insertions(+), 43 deletions(-)
New commits: commit 845554a6a3a76dc4161409f9d6648b8ff4133068 Author: Gülşah Köse <gulsah.k...@collabora.com> AuthorDate: Tue Sep 1 20:15:30 2020 +0300 Commit: Gülşah Köse <gulsah.k...@collabora.com> CommitDate: Tue Sep 1 23:17:03 2020 +0200 Revert "wsd: parse headers with Poco::MessageHeader" This reverts commit dbc562d9abc997b196fd6d4e5e71f42d442488d0. tst-05694-05694 2020-08-26 12:59:14.343136 [ unittest ] ERR Invalid HTTP header [def]: Malformed message: Field name too long/no colon found| ../common/Util.cpp:980 Following part of the code tests a request with corrupted http header: Authorization auth2(Authorization::Type::Header, "def"); Poco::Net::HTTPRequest req2; auth2.authorizeRequest(req2); LOK_ASSERT(!req2.has("Authorization")); Poco library throws exception. Change-Id: Ic31a80c0e1e325de27c23059e2bcb3f00d39ad16 Reviewed-on: https://gerrit.libreoffice.org/c/online/+/101887 Tested-by: Jenkins Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Gülşah Köse <gulsah.k...@collabora.com> diff --git a/common/Authorization.cpp b/common/Authorization.cpp index 5613aa91a..93c5704fc 100644 --- a/common/Authorization.cpp +++ b/common/Authorization.cpp @@ -45,15 +45,42 @@ void Authorization::authorizeRequest(Poco::Net::HTTPRequest& request) const switch (_type) { case Type::Token: - Util::setHttpHeaders(request, "Authorization: Bearer " + _data); - assert(request.has("Authorization") && "HTTPRequest missing Authorization header"); + request.set("Authorization", "Bearer " + _data); break; case Type::Header: + { // there might be more headers in here; like // Authorization: Basic .... // X-Something-Custom: Huh - Util::setHttpHeaders(request, _data); + // Split based on \n's or \r's and trim, to avoid nonsense in the + // headers + StringVector tokens(Util::tokenizeAnyOf(_data, "\n\r")); + for (auto it = tokens.begin(); it != tokens.end(); ++it) + { + std::string token = tokens.getParam(*it); + + size_t separator = token.find_first_of(':'); + if (separator != std::string::npos) + { + size_t headerStart = token.find_first_not_of(' ', 0); + size_t headerEnd = token.find_last_not_of(' ', separator - 1); + + size_t valueStart = token.find_first_not_of(' ', separator + 1); + size_t valueEnd = token.find_last_not_of(' '); + + // set the header + if (headerStart != std::string::npos && headerEnd != std::string::npos && + valueStart != std::string::npos && valueEnd != std::string::npos) + { + size_t headerLength = headerEnd - headerStart + 1; + size_t valueLength = valueEnd - valueStart + 1; + + request.set(token.substr(headerStart, headerLength), token.substr(valueStart, valueLength)); + } + } + } break; + } default: // assert(false); throw BadRequestException("Invalid HTTP request type"); diff --git a/common/Util.cpp b/common/Util.cpp index f1cd61b69..e0ce00250 100644 --- a/common/Util.cpp +++ b/common/Util.cpp @@ -59,7 +59,7 @@ #include <Poco/Util/Application.h> #include "Common.hpp" -#include <common/Log.hpp> +#include "Log.hpp" #include "Protocol.hpp" #include "Util.hpp" @@ -953,38 +953,6 @@ namespace Util return std::ctime(&t); } - void setHttpHeaders(Poco::Net::HTTPRequest& request, const std::string& headers) - { - // Look for either \r or \n and replace them with a single \r\n - // as prescribed by rfc2616 as valid header delimeter, removing - // any invalid line breaks, to avoid nonsense in the headers. - const StringVector tokens = Util::tokenizeAnyOf(headers, "\n\r"); - const std::string header = tokens.cat("\r\n", 0); - try - { - // Now parse to preserve folded headers and other - // corner cases that is conformant to the rfc, - // detecting any errors and/or invalid entries. - // NB: request.read() expects full message and will fail. - Poco::Net::MessageHeader msgHeader; - std::istringstream iss(header); - msgHeader.read(iss); - for (const auto& entry : msgHeader) - { - // Set each header entry. - request.set(Util::trimmed(entry.first), Util::trimmed(entry.second)); - } - } - catch (const Poco::Exception& ex) - { - LOG_ERR("Invalid HTTP header [" << header << "]: " << ex.displayText()); - } - catch (const std::exception& ex) - { - LOG_ERR("Invalid HTTP header [" << header << "]: " << ex.what()); - } - } - bool isFuzzing() { #if LIBFUZZER diff --git a/common/Util.hpp b/common/Util.hpp index bb81cfb75..9dbfebe8b 100644 --- a/common/Util.hpp +++ b/common/Util.hpp @@ -35,7 +35,6 @@ #include <Poco/File.h> #include <Poco/Path.h> #include <Poco/RegularExpression.h> -#include <Poco/Net/HTTPRequest.h> #define LOK_USE_UNSTABLE_API #include <LibreOfficeKit/LibreOfficeKitEnums.h> @@ -1083,12 +1082,6 @@ int main(int argc, char**argv) /// conversion from steady_clock for debugging / tracing std::string getSteadyClockAsString(const std::chrono::steady_clock::time_point &time); - /// Set the request header by splitting multiple entries by \r or \n. - /// Needed to sanitize user-provided http headers, after decoding. - /// This is much more tolerant to line breaks than the rfc allows. - /// Note: probably should move to a more appropriate home. - void setHttpHeaders(Poco::Net::HTTPRequest& request, const std::string& headers); - /// Automatically execute code at end of current scope. /// Used for exception-safe code. class ScopeGuard _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits