From 8683ee08e7755e5a750f265b0387bfdfe144ea42 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Tue, 25 Oct 2022 15:56:53 +0200 Subject: [PATCH 1/4] Validate edit locally token before sending to server Signed-off-by: Claudio Cambra --- src/gui/folderman.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 00818e80a8..da1785897f 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -1513,7 +1513,18 @@ void FolderMan::editFileLocally(const QString &userId, const QString &relPath, c showError(accountFound, tr("Could not find a folder to sync."), relPath); return; } - + + // Token is an alphanumeric string 128 chars long. + // Ensure that is what we received and what we are sending to the server. + const QRegularExpression tokenRegex("^[a-zA-Z0-9]{128}$"); + const auto regexMatch = tokenRegex.match(token); + + // Means invalid token type received, be cautious with bad token + if(!regexMatch.hasMatch()) { + showError(accountFound, tr("Invalid token received."), tr("Please try again.")); + return; + } + const auto relPathSplit = relPath.split(QLatin1Char('/')); if (relPathSplit.size() > 0) { Systray::instance()->createEditFileLocallyLoadingDialog(relPathSplit.last()); @@ -1522,7 +1533,9 @@ void FolderMan::editFileLocally(const QString &userId, const QString &relPath, c return; } - const auto checkTokenForEditLocally = new SimpleApiJob(accountFound->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(token)); + // Sanitise the token + const auto encodedToken = QString(QUrl::toPercentEncoding(token)); + const auto checkTokenForEditLocally = new SimpleApiJob(accountFound->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); checkTokenForEditLocally->setVerb(SimpleApiJob::Verb::Post); checkTokenForEditLocally->setBody(QByteArray{"path=/"}.append(relPath.toUtf8())); connect(checkTokenForEditLocally, &SimpleApiJob::resultReceived, checkTokenForEditLocally, [this, folderForFile, localFilePath, showError, accountFound, relPath] (int statusCode) { From 0c6127de44b0d537cfe232caf08940f792e2bcfb Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Tue, 25 Oct 2022 19:19:43 +0200 Subject: [PATCH 2/4] Make sure to encode the relPath Signed-off-by: Claudio Cambra --- src/gui/folderman.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index da1785897f..412910f941 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -1534,10 +1534,12 @@ void FolderMan::editFileLocally(const QString &userId, const QString &relPath, c } // Sanitise the token - const auto encodedToken = QString(QUrl::toPercentEncoding(token)); + const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(token)); + // Sanitise the relPath + const auto encodedRelPath = QUrl::toPercentEncoding(relPath); const auto checkTokenForEditLocally = new SimpleApiJob(accountFound->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); checkTokenForEditLocally->setVerb(SimpleApiJob::Verb::Post); - checkTokenForEditLocally->setBody(QByteArray{"path=/"}.append(relPath.toUtf8())); + checkTokenForEditLocally->setBody(QByteArray{"path=/"}.append(encodedRelPath)); connect(checkTokenForEditLocally, &SimpleApiJob::resultReceived, checkTokenForEditLocally, [this, folderForFile, localFilePath, showError, accountFound, relPath] (int statusCode) { constexpr auto HTTP_OK_CODE = 200; if (statusCode != HTTP_OK_CODE) { From 0b33ce2b40285d87150abd745a1ad90661be6c81 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Wed, 26 Oct 2022 16:07:59 +0200 Subject: [PATCH 3/4] Make sure to check relPath and compare to canonical cleaned path Signed-off-by: Claudio Cambra --- src/gui/folderman.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 412910f941..46e586b89b 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -1488,6 +1488,27 @@ void FolderMan::editFileLocally(const QString &userId, const QString &relPath, c return; } + // We want to check that the path is canonical and not relative + // (i.e. that it doesn't contain ../../) but we always receive + // a relative path, so let's make it absolute by prepending a + // slash + + auto slashPrefixedPath = relPath; + if (!slashPrefixedPath.startsWith('/')) { + slashPrefixedPath.prepend('/'); + } + + // Let's check that the filepath is canonical, and that the request + // contains no funny behaviour regarding paths + const auto cleanedPath = QDir::cleanPath(slashPrefixedPath); + + if (cleanedPath != slashPrefixedPath) { + qCWarning(lcFolderMan) << "Provided relPath was:" << relPath + << "which is not canonical (cleaned path was:" << cleanedPath << ")"; + showError(accountFound, tr("Invalid file path was provided."), tr("Please try again.")); + return; + } + const auto foundFiles = findFileInLocalFolders(relPath, accountFound->account()); if (foundFiles.isEmpty()) { From 734c986cd1234ed9e5c54e3cc03294ee11cf7e40 Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Thu, 27 Oct 2022 16:32:51 +0200 Subject: [PATCH 4/4] Use addQueryParams for checkTokenForEditLocally SimpleApiJob Signed-off-by: Claudio Cambra --- src/gui/folderman.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 46e586b89b..44bc904d64 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -1554,13 +1554,14 @@ void FolderMan::editFileLocally(const QString &userId, const QString &relPath, c return; } - // Sanitise the token - const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(token)); - // Sanitise the relPath - const auto encodedRelPath = QUrl::toPercentEncoding(relPath); + const auto encodedToken = QString::fromUtf8(QUrl::toPercentEncoding(token)); // Sanitise the token + const auto encodedRelPath = QUrl::toPercentEncoding(slashPrefixedPath); // Sanitise the relPath const auto checkTokenForEditLocally = new SimpleApiJob(accountFound->account(), QStringLiteral("/ocs/v2.php/apps/files/api/v1/openlocaleditor/%1").arg(encodedToken)); + + QUrlQuery params; + params.addQueryItem(QStringLiteral("path"), slashPrefixedPath); + checkTokenForEditLocally->addQueryParams(params); checkTokenForEditLocally->setVerb(SimpleApiJob::Verb::Post); - checkTokenForEditLocally->setBody(QByteArray{"path=/"}.append(encodedRelPath)); connect(checkTokenForEditLocally, &SimpleApiJob::resultReceived, checkTokenForEditLocally, [this, folderForFile, localFilePath, showError, accountFound, relPath] (int statusCode) { constexpr auto HTTP_OK_CODE = 200; if (statusCode != HTTP_OK_CODE) {