diff --git a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php index 25b5ceefdf1..64a3845e1e2 100644 --- a/apps/dav/lib/Files/Sharing/FilesDropPlugin.php +++ b/apps/dav/lib/Files/Sharing/FilesDropPlugin.php @@ -8,6 +8,7 @@ namespace OCA\DAV\Files\Sharing; use OC\Files\View; use OCP\Share\IShare; +use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\MethodNotAllowed; use Sabre\DAV\ServerPlugin; use Sabre\HTTP\RequestInterface; @@ -65,14 +66,28 @@ class FilesDropPlugin extends ServerPlugin { // Extract the attributes for the file request $isFileRequest = false; $attributes = $this->share->getAttributes(); - $nickName = $request->hasHeader('X-NC-Nickname') ? urldecode($request->getHeader('X-NC-Nickname')) : null; + $nickName = $request->hasHeader('X-NC-Nickname') ? trim(urldecode($request->getHeader('X-NC-Nickname'))) : null; if ($attributes !== null) { $isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true; } // We need a valid nickname for file requests - if ($isFileRequest && ($nickName == null || trim($nickName) === '')) { - throw new MethodNotAllowed('Nickname is required for file requests'); + if ($isFileRequest && !$nickName) { + throw new BadRequest('Nickname is required for file requests'); + } + + if ($nickName !== null) { + try { + $this->view->verifyPath($path, $nickName); + } catch (\Exception $e) { + // If the path is not valid, we throw an exception + throw new BadRequest('Invalid nickname: ' . $nickName); + } + + // Forbid nicknames starting with a dot + if (str_starts_with($nickName, '.')) { + throw new BadRequest('Invalid nickname: ' . $nickName); + } } // If this is a file request we need to create a folder for the user diff --git a/build/integration/filesdrop_features/filesdrop.feature b/build/integration/filesdrop_features/filesdrop.feature index 2c9156dea02..7a8b3ea4aef 100644 --- a/build/integration/filesdrop_features/filesdrop.feature +++ b/build/integration/filesdrop_features/filesdrop.feature @@ -47,7 +47,7 @@ Feature: FilesDrop And Downloading file "/drop/a.txt" Then Downloaded content should be "abc" - Scenario: Files drop forbis MKCOL + Scenario: Files drop forbid MKCOL Given user "user0" exists And As an "user0" And user "user0" created a folder "/drop" @@ -90,3 +90,42 @@ Feature: FilesDrop Then Downloaded content should be "abc" And Downloading file "/drop/Mallory/a (2).txt" Then Downloaded content should be "def" + + Scenario: Files request drop with invalid nickname with slashes + Given user "user0" exists + And As an "user0" + And user "user0" created a folder "/drop" + And as "user0" creating a share with + | path | drop | + | shareType | 4 | + | permissions | 4 | + | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] | + | shareWith | | + When Dropping file "/folder/a.txt" with "abc" as "Alice/Bob/Mallory" + Then the HTTP status code should be "400" + + Scenario: Files request drop with invalid nickname with forbidden characters + Given user "user0" exists + And As an "user0" + And user "user0" created a folder "/drop" + And as "user0" creating a share with + | path | drop | + | shareType | 4 | + | permissions | 4 | + | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] | + | shareWith | | + When Dropping file "/folder/a.txt" with "abc" as ".htaccess" + Then the HTTP status code should be "400" + + Scenario: Files request drop with invalid nickname with forbidden characters + Given user "user0" exists + And As an "user0" + And user "user0" created a folder "/drop" + And as "user0" creating a share with + | path | drop | + | shareType | 4 | + | permissions | 4 | + | attributes | [{"scope":"fileRequest","key":"enabled","value":true}] | + | shareWith | | + When Dropping file "/folder/a.txt" with "abc" as ".Mallory" + Then the HTTP status code should be "400"