fix: do not accept federated shares where the name is too long

[DeepDiver1975]

Description

Federated shares with a too long name can result in in accessible shares on the receiving server

Related Issue

• Fixes <issue_link>

Motivation and Context

How Has This Been Tested?

• test environment:
• test case 1:
• test case 2:
• ...

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[ownclouders]

💥 Acceptance tests pipeline apiProxySmoke-8-8-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/38203/174

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

100.0% Coverage
0.0% Duplication

[DeepDiver1975]

@jvillafanez please review again - thx

[phil-davis]

@jnweiger @pako81 is this something that is wanted for 10.12.1 release?
If so, then you will need to cherry-pick it into release-10.12.1 branch.

[saw-jan]

I don't know how to confirm this.
Steps that I tried:

1. server1, mount an external storage (SMB) - since creating long name file is not possible from server UI
2. create a file with long name in the SMB share folder
   aQuickBrownFoxJumpsOverAVeryLazyDog-aQuickBrownFoxJumpsOverAVeryLazyDog-aQuickBrownFoxJumpsOverAVeryLazyDog-aQuickBrownFoxJumpsOverAVeryLazyDog-aQuickBrownFoxJumpsOverAVeryLazyDog-aQuickBrownFoxJumpsOverAVeryLazyDog-aQuickBrownFo.txt
3. server1, do a federate share of that file to another server (10.13.0) - admin@<latest-server-url>
4. server2, accept the share - (accept works)
5. server2, open the file - (content is there)

From desktop-client:
4. add server2 account to the desktop-client
5. accept the share form activity window
6. share is there

CC @DeepDiver1975 @jvillafanez

[jnweiger]

Not exaclty sure, where the length limit is. drone CI uses a 290 char string to trigger this. Yours is shorter: 233 chars.

[pako81]

The verifyPath method in View.php is used:

https://github.com/owncloud/core/blob/master/lib/private/Files/View.php#L1921

which is then calling:

https://github.com/owncloud/core/blob/master/lib/private/Files/Storage/Common.php#L505-L511

where a FileNameTooLongException exception is thrown if the path is longer than 255 chars.

[jesmrec]

where a FileNameTooLongException exception is thrown if the path is longer than 255 chars.

but, clients (including web) do not allow such amount of characters in the filename. So, that exception is never thrown.

[jnweiger]

The verifyPath method in View.php is used:
https://github.com/owncloud/core/blob/master/lib/private/Files/View.php#L1921>
which is then calling:
https://github.com/owncloud/core/blob/master/lib/private/Files/Storage/Common.php#L505-L511
where a FileNameTooLongException exception is thrown if the path is longer than 255 chars.

Ouch. This will explode, when versioning or trashcan add their suffixes to the filename.

./files_versions/Documents/LoremIpsum.txt.v1692195415
./files_trashbin/files/Diagramm.drawio.d1692184476

[pako81]

Ouch. This will explode, when versioning or trashcan add their suffixes to the filename.
./files_versions/Documents/LoremIpsum.txt.v1692195415
./files_trashbin/files/Diagramm.drawio.d1692184476

not related to this specific issue but, yes, we need to check what happens with occ files:scan when scanning versions and trashbin paths longer than 255 chars.

[jnweiger]

The code path with these checks is not executed.
Patched apps/federatedfilesharing/lib/Controller/RequestHandlerController.php

        public function createShare() {
                \OCP\Util::writeLog('federatedfilesharing', 'JW was here createShare ', \OCP\Util::ERROR);
                throw new BadRequestException('JW was here. createShare');
                try {

...

       public function acceptShare($id) {
                \OCP\Util::writeLog('federatedfilesharing', 'JW was here acceptShare ', \OCP\Util::ERROR);
                throw new BadRequestException('JW was here. acceptShare');
                try {

...

• Incoming federated shares can still be accepted. 😱

[DeepDiver1975]

I don't know how to confirm this.

In a regular use case this will not happen because files on all servers have the correct file name length.

This is a safety net against wrong usage of the http request. Submit an http request using curl.

[saw-jan]

Tried to re-check:

1. Created file aQuickBrownFoxJumpsOverAVeryLazyDog-aQuickBrownFoxJumpsOverAVeryLazyDog-aQuickBr.txt (80 chars)
2. Edited here to have only 55

   core/lib/private/Files/Storage/Common.php

   Lines 505 to 511 in 71e0ebe

   	public function verifyPath($path, $fileName) {
   	if (isset($fileName[255])) {
   	throw new FileNameTooLongException();
   	}
   	$this->verifyPosixPath($fileName);
   	}

3. Create a fed share

   curl -XPOST "http://localhost/oc10/ocs/v1.php/apps/files_sharing/api/v1/shares?format=json" \
   -d"shareType=6&shareWith=admin@localhost/core&permissions=19&path=/aQuickBrownFoxJumpsOverAVeryLazyDog-aQuickBrownFoxJumpsOverAVeryLazyDog-aQuickBraisdiusadsagdiasdasd.txt" \
   -uadmin:admin

4. Share is created (:exclamation:)

NOTE: The share request doesn't hit the createShare controller in

core/apps/federatedfilesharing/lib/Controller/RequestHandlerController.php

Lines 99 to 100 in 71e0ebe

	public function createShare() {
	try {

but hits

core/apps/files_sharing/lib/Controller/Share20OcsController.php

Lines 372 to 373 in 71e0ebe

	public function createShare() {
	$share = $this->shareManager->newShare();

[DeepDiver1975]

curl -XPOST "http://localhost/oc10/ocs/v1.php/apps/files_sharing/api/v1/shares?format=json" \

federatedfilesharing is the app -> not files_sharing ....

[DeepDiver1975]

4. Share is created (❗)

worth an issue for further investigation