Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate and sanitise edit locally token and relpath before sending to server #5093

Merged
merged 4 commits into from
Oct 28, 2022
Merged

Validate and sanitise edit locally token and relpath before sending to server #5093

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8683ee0
Validate edit locally token before sending to server
claucambra Oct 25, 2022
0c6127d
Make sure to encode the relPath
claucambra Oct 25, 2022
0b33ce2
Make sure to check relPath and compare to canonical cleaned path
claucambra Oct 26, 2022
734c986
Use addQueryParams for checkTokenForEditLocally SimpleApiJob
claucambra Oct 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 40 additions & 3 deletions src/gui/folderman.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -1513,7 +1534,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());
Expand All @@ -1522,9 +1554,14 @@ 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));
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(relPath.toUtf8()));
connect(checkTokenForEditLocally, &SimpleApiJob::resultReceived, checkTokenForEditLocally, [this, folderForFile, localFilePath, showError, accountFound, relPath] (int statusCode) {
constexpr auto HTTP_OK_CODE = 200;
if (statusCode != HTTP_OK_CODE) {
Expand Down