Skip to content

[stable21] Check for invalid characters before trimming #29907

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

Merged
merged 3 commits into from
Nov 26, 2021
Merged

[stable21] Check for invalid characters before trimming #29907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f700abc
Check for invalid characters before trimming
nickvergessen Nov 25, 2021
933a967
Add an integration test
nickvergessen Nov 25, 2021
9cc47c5
Correctly set the response after a ClientException as well
nickvergessen Nov 25, 2021
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
28 changes: 22 additions & 6 deletions build/integration/features/bootstrap/WebDav.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,10 @@ public function userUploadsAFileTo($user, $source, $destination) {
try {
$this->response = $this->makeDavRequest($user, "PUT", $destination, [], $file);
} catch (\GuzzleHttp\Exception\ServerException $e) {
// 4xx and 5xx responses cause an exception
// 5xx responses cause a server exception
$this->response = $e->getResponse();
} catch (\GuzzleHttp\Exception\ClientException $e) {
// 4xx responses cause a client exception
$this->response = $e->getResponse();
}
}
Expand Down Expand Up @@ -488,7 +491,10 @@ public function userUploadsAFileWithContentTo($user, $content, $destination) {
try {
$this->response = $this->makeDavRequest($user, "PUT", $destination, [], $file);
} catch (\GuzzleHttp\Exception\ServerException $e) {
// 4xx and 5xx responses cause an exception
// 5xx responses cause a server exception
$this->response = $e->getResponse();
} catch (\GuzzleHttp\Exception\ClientException $e) {
// 4xx responses cause a client exception
$this->response = $e->getResponse();
}
}
Expand All @@ -503,7 +509,10 @@ public function userDeletesFile($user, $type, $file) {
try {
$this->response = $this->makeDavRequest($user, 'DELETE', $file, []);
} catch (\GuzzleHttp\Exception\ServerException $e) {
// 4xx and 5xx responses cause an exception
// 5xx responses cause a server exception
$this->response = $e->getResponse();
} catch (\GuzzleHttp\Exception\ClientException $e) {
// 4xx responses cause a client exception
$this->response = $e->getResponse();
}
}
Expand All @@ -518,7 +527,10 @@ public function userCreatedAFolder($user, $destination) {
$destination = '/' . ltrim($destination, '/');
$this->response = $this->makeDavRequest($user, "MKCOL", $destination, []);
} catch (\GuzzleHttp\Exception\ServerException $e) {
// 4xx and 5xx responses cause an exception
// 5xx responses cause a server exception
$this->response = $e->getResponse();
} catch (\GuzzleHttp\Exception\ClientException $e) {
// 4xx responses cause a client exception
$this->response = $e->getResponse();
}
}
Expand Down Expand Up @@ -589,8 +601,12 @@ public function userMovesNewChunkFileWithIdToMychunkedfileWithSize($user, $id, $
public function downloadingFileAs($fileName, $user) {
try {
$this->response = $this->makeDavRequest($user, 'GET', $fileName, []);
} catch (\GuzzleHttp\Exception\ServerException $ex) {
$this->response = $ex->getResponse();
} catch (\GuzzleHttp\Exception\ServerException $e) {
// 5xx responses cause a server exception
$this->response = $e->getResponse();
} catch (\GuzzleHttp\Exception\ClientException $e) {
// 4xx responses cause a client exception
$this->response = $e->getResponse();
}
}

Expand Down
9 changes: 9 additions & 0 deletions build/integration/features/webdav-related.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,3 +608,12 @@ Feature: webdav-related
And user "user0" uploads new chunk file "3" with "CCCCC" to id "chunking-42"
When user "user0" moves new chunk file with id "chunking-42" to "/myChunkedFile.txt" with size 15
Then the HTTP status code should be "201"

Scenario: Creating a folder with invalid characters
Given using new dav path
And As an "admin"
And user "user0" exists
And user "user1" exists
And As an "user1"
And user "user1" created a folder "/testshare "
Then the HTTP status code should be "400"
2 changes: 1 addition & 1 deletion lib/private/Files/Storage/Common.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,8 @@ public function verifyPath($path, $fileName) {
* @throws InvalidPathException
*/
protected function verifyPosixPath($fileName) {
$fileName = trim($fileName);
$this->scanForInvalidCharacters($fileName, "\\/");
$fileName = trim($fileName);
$reservedNames = ['*'];
if (in_array($fileName, $reservedNames)) {
throw new ReservedWordException();
Expand Down