haraldwolff
/
collabora-online
mirror of https://github.com/CollaboraOnline/online.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

wsd: detect unencoded WOPISrc 

We should always have an encoded WOPISrc.
We add detection logic to make sure
that all URIs that contain WOPISrc have
it encoded properly. We do this by
comparing the decoded WOPISrc with
the original URI.

Change-Id: Ia0c2a79b009ce105321ad35db3d4f81006e81cb3
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
pull/8125/head
Ashod Nakashian 2024-02-07 21:03:30 -05:00 committed by Ashod Nakashian
parent a809541878
commit d3905698b0
8 changed files with 113 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
common/Util.cpp
View File

@ -747,6 +747,22 @@ namespace Util
return decoded;
}
bool needsURIEncoding(const std::string& uri, const std::string& reserved)
{
const std::string decoded = decodeURIComponent(uri);
if (decoded != uri)
{
// We could decode it; must have been encoded already.
return false;
}
// Identical when decoded, might need encoding.
const std::string encoded = encodeURIComponent(uri, reserved);
// If identical, then doesn't need encoding.
return encoded != uri;
}
/// Split a string in two at the delimiter and give the delimiter to the first.
static
std::pair<std::string, std::string> splitLast2(const std::string& str, const char delimiter = ' ')

8
common/Util.hpp
View File

@ -1097,6 +1097,14 @@ int main(int argc, char**argv)
/// Decode a URI encoded with encodeURIComponent.
std::string decodeURIComponent(const std::string& uri);
/// Checks whether or not the given string is encoded.
/// That is, a string that is identical when encoded
/// will return false. Similarly, a string that is
/// already encoded will return false.
/// Optionally takes a string of reserved characters
/// to escape while encoding.
bool needsURIEncoding(const std::string& uri, const std::string& reserved = ",/?:@&=+$#");
/// Cleanup a filename replacing anything potentially problematic
/// either for a URL or for a file path
std::string cleanupFilename(const std::string &filename);

43
net/HttpHelper.hpp
View File

@ -11,6 +11,7 @@
#pragma once
#include "Util.hpp"
#include <memory>
#include <string>
@ -35,6 +36,48 @@ void sendFileAndShutdown(const std::shared_ptr<StreamSocket>& socket, const std:
http::Response& response,
bool noCache = false, bool deflate = false, const bool headerOnly = false);
/// Verifies that the given WOPISrc is properly URI-encoded.
/// Warns if it isn't and, in debug builds, closes the socket (if given) and returns false.
/// The idea is to only warn in release builds, but to help developers in debug builds.
/// Returns false only in debug build.
inline bool verifyWOPISrc(const std::string& uri, const std::string& wopiSrc,
const std::shared_ptr<StreamSocket>& socket = {})
{
// getQueryParameters(), which is used to extract wopiSrc, decodes the values.
// Compare with the URI. WopiSrc is complex enough to require encoding.
// But, if it matches, check if the WOPISrc actually needed encoding.
if (uri.find(wopiSrc) != std::string::npos && Util::needsURIEncoding(wopiSrc))
{
#if !ENABLE_DEBUG
(void)socket;
static bool warnedOnce = false;
if (!warnedOnce)
{
LOG_WRN_S("WOPISrc validation error: unencoded WOPISrc ["
<< wopiSrc << "] in URL [" << uri
<< "]. WOPISrc must be URI-encoded. This is highly problematic with proxies, "
"load balancers, and when tunneling. Will not warn again");
warnedOnce = true;
}
#else
// In debug mode, be assertive. Logs might go unnoticed.
LOG_ERR_S("WOPISrc validation error: unencoded WOPISrc ["
<< wopiSrc << "] in URL [" << uri
<< "]. This is highly problematic with proxies, load balancers, and when "
"tunneling");
if (socket)
{
sendErrorAndShutdown(http::StatusCode::BadRequest, socket,
"WOPISrc must be URI-encoded");
}
return false;
#endif // ENABLE_DEBUG
}
return true;
}
} // namespace HttpHelper
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */

28
test/RequestDetailsTests.cpp
View File

@ -681,19 +681,19 @@ void RequestDetailsTests::testRequestDetails()
}
{
static const std::string URI
= "/cool/"
"https%3A%2F%2Fexample.com%3A8443%2Frest%2Ffiles%2Fwopi%2Ffiles%"
"2F8ac75551de4d89e60002%3Faccess_header%3DAuthorization%253A%252520Bearer%"
"252520poiuytrewq%25250D%25250A%25250D%25250AX-Requested-"
"With%253A%252520XMLHttpRequest%26reuse_cookies%3Dlang%253Den-us%253A_ga_"
"LMX4TVJ02K%253DGS1.1%"
"253AToken%253DeyJhbGciOiJIUzUxMiJ9.vajknfkfajksdljfiwjek-"
"W90fmgVb3C-00-eSkJBDqDNSYA%253APublicToken%"
"253Dabc%253AZNPCQ003-32383700%253De9c71c3b%"
"253AJSESSIONID%253Dnode0.node0%26permission%3Dedit/"
"ws?WOPISrc=https://example.com:8443/rest/files/wopi/files/"
"8c74c1deff7dede002&compat=/ws";
static const std::string URI =
"/cool/"
"https%3A%2F%2Fexample.com%3A8443%2Frest%2Ffiles%2Fwopi%2Ffiles%"
"2F8ac75551de4d89e60002%3Faccess_header%3DAuthorization%253A%252520Bearer%"
"252520poiuytrewq%25250D%25250A%25250D%25250AX-Requested-"
"With%253A%252520XMLHttpRequest%26reuse_cookies%3Dlang%253Den-us%253A_ga_"
"LMX4TVJ02K%253DGS1.1%"
"253AToken%253DeyJhbGciOiJIUzUxMiJ9.vajknfkfajksdljfiwjek-"
"W90fmgVb3C-00-eSkJBDqDNSYA%253APublicToken%"
"253Dabc%253AZNPCQ003-32383700%253De9c71c3b%"
"253AJSESSIONID%253Dnode0.node0%26permission%3Dedit/"
"ws?WOPISrc=https%3A%2F%2Fexample.com%3A8443%2Frest%2Ffiles%2Fwopi%2Ffiles%"
"2F8c74c1deff7dede002&compat=/ws";
Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET, URI,
Poco::Net::HTTPMessage::HTTP_1_1);
@ -736,7 +736,7 @@ void RequestDetailsTests::testRequestDetails()
const std::string permission = "edit";
LOK_ASSERT_EQUAL(permission, it != params.end() ? it->second : "");
LOK_ASSERT_EQUAL(static_cast<std::size_t>(11), details.size());
LOK_ASSERT_EQUAL(static_cast<std::size_t>(5), details.size());
LOK_ASSERT_EQUAL(std::string("cool"), details[0]);
LOK_ASSERT(details.equals(0, "cool"));

13
wsd/COOLWSD.cpp
View File

@ -14,6 +14,7 @@
#include "COOLWSD.hpp"
#include "ProofKey.hpp"
#include "RequestDetails.hpp"
#if ENABLE_FEATURE_LOCK
#include "CommandControl.hpp"
#endif
@ -4626,6 +4627,12 @@ private:
return;
}
// Verify that the WOPISrc is properly encoded.
if (!HttpHelper::verifyWOPISrc(request.getURI(), WOPISrc, socket))
{
return;
}
const auto docKey = RequestDetails::getDocKey(WOPISrc);
LOG_TRC_S("Clipboard request for us: [" << serverId << "] with tag [" << tag
<< "] on docKey [" << docKey << ']');
@ -4761,6 +4768,12 @@ private:
return;
}
// Verify that the WOPISrc is properly encoded.
if (!HttpHelper::verifyWOPISrc(request.getURI(), WOPISrc, socket))
{
return;
}
const auto docKey = RequestDetails::getDocKey(WOPISrc);
LOG_TRC_S("Looking up DocBroker with docKey [" << docKey << "] referenced in WOPISrc ["
<< WOPISrc

5
wsd/FileServer.cpp
View File

@ -1362,6 +1362,11 @@ void FileServerRequestHandler::preprocessFile(const HTTPRequest& request,
{
if (param.first == "WOPISrc")
{
if (!HttpHelper::verifyWOPISrc(request.getURI(), param.second, socket))
{
return;
}
const Poco::URI uriWopiFrameAncestor(Util::decodeURIComponent(param.second));
// Remove parameters from URL
const std::string& wopiFrameAncestor = uriWopiFrameAncestor.getHost();

11
wsd/RequestDetails.cpp
View File

@ -18,6 +18,7 @@
#include "HostUtil.hpp"
#include <Poco/URI.h>
#include <stdexcept>
#include "Exceptions.hpp"
namespace
@ -28,6 +29,16 @@ std::map<std::string, std::string> getParams(const std::string& uri)
std::map<std::string, std::string> result;
for (const auto& param : Poco::URI(uri).getQueryParameters())
{
// getQueryParameters() decodes the values. Compare with the URI.
if (param.first == "WOPISrc" && uri.find(param.second) != std::string::npos)
{
LOG_WRN("WOPISrc validation error: unencoded WOPISrc [" << param.second
<< "] in URL: " << uri);
#if ENABLE_DEBUG
throw std::runtime_error("WOPISrc must be URI-encoded");
#endif // ENABLE_DEBUG
}
std::string key = Util::decodeURIComponent(param.first);
std::string value = Util::decodeURIComponent(param.second);
LOG_TRC("Decoding param [" << param.first << "] = [" << param.second << "] -> [" << key

3
wsd/RequestDetails.hpp
View File

@ -143,6 +143,9 @@ public:
/// Sanitize the URI and return the document-specific key.
static std::string getDocKey(const std::string& uri) { return getDocKey(sanitizeURI(uri)); }
/// Returns false if the WOPISrc is not encoded correctly.
static bool validateWOPISrc(const std::string& uri) { return !Util::needsURIEncoding(uri); }
// matches the WOPISrc if used. For load balancing
// must be 2nd element in the path after /cool/<here>
std::string getLegacyDocumentURI() const { return getField(Field::LegacyDocumentURI); }