0
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2025-02-25 17:30:20 +00:00

fix(files): Harden thumbnail endpoint

- Catch all thrown exceptions and handle in such a way you do not get
  information about forbidden files.
- Resepect download permissions of shares.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
This commit is contained in:
Ferdinand Thiessen 2025-01-25 19:46:19 +01:00
parent 6b6401f9c8
commit 08319ad5a6
No known key found for this signature in database
GPG key ID: 45FAE7268762B400
2 changed files with 103 additions and 7 deletions
apps/files
lib/Controller
tests/Controller

View file

@ -29,8 +29,11 @@ use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Http\StreamResponse;
use OCP\Files\File;
use OCP\Files\Folder;
use OCP\Files\InvalidPathException;
use OCP\Files\IRootFolder;
use OCP\Files\NotFoundException;
use OCP\Files\NotPermittedException;
use OCP\Files\Storage\ISharedStorage;
use OCP\Files\StorageNotAvailableException;
use OCP\IConfig;
use OCP\IL10N;
@ -92,20 +95,28 @@ class ApiController extends Controller {
}
try {
$file = $this->userFolder->get($file);
if ($file instanceof Folder) {
$file = $this->userFolder?->get($file);
if ($file === null
|| !($file instanceof File)
|| ($file->getId() <= 0)
) {
throw new NotFoundException();
}
if ($file->getId() <= 0) {
return new DataResponse(['message' => 'File not found.'], Http::STATUS_NOT_FOUND);
// Validate the user is allowed to download the file (preview is some kind of download)
$storage = $file->getStorage();
if ($storage->instanceOfStorage(ISharedStorage::class)) {
/** @var ISharedStorage $storage */
$attributes = $storage->getShare()->getAttributes();
if ($attributes !== null && $attributes->getAttribute('permissions', 'download') === false) {
throw new NotFoundException();
}
}
/** @var File $file */
$preview = $this->previewManager->getPreview($file, $x, $y, true);
return new FileDisplayResponse($preview, Http::STATUS_OK, ['Content-Type' => $preview->getMimeType()]);
} catch (NotFoundException $e) {
} catch (NotFoundException|NotPermittedException|InvalidPathException) {
return new DataResponse(['message' => 'File not found.'], Http::STATUS_NOT_FOUND);
} catch (\Exception $e) {
return new DataResponse([], Http::STATUS_BAD_REQUEST);

View file

@ -19,6 +19,8 @@ use OCP\Files\Folder;
use OCP\Files\IRootFolder;
use OCP\Files\NotFoundException;
use OCP\Files\SimpleFS\ISimpleFile;
use OCP\Files\Storage\ISharedStorage;
use OCP\Files\Storage\IStorage;
use OCP\Files\StorageNotAvailableException;
use OCP\IConfig;
use OCP\IL10N;
@ -26,7 +28,9 @@ use OCP\IPreview;
use OCP\IRequest;
use OCP\IUser;
use OCP\IUserSession;
use OCP\Share\IAttributes;
use OCP\Share\IManager;
use OCP\Share\IShare;
use Psr\Log\LoggerInterface;
use Test\TestCase;
@ -173,8 +177,12 @@ class ApiControllerTest extends TestCase {
}
public function testGetThumbnailInvalidImage(): void {
$storage = $this->createMock(IStorage::class);
$storage->method('instanceOfStorage')->with(ISharedStorage::class)->willReturn(false);
$file = $this->createMock(File::class);
$file->method('getId')->willReturn(123);
$file->method('getStorage')->willReturn($storage);
$this->userFolder->method('get')
->with($this->equalTo('unknown.jpg'))
->willReturn($file);
@ -196,9 +204,86 @@ class ApiControllerTest extends TestCase {
$this->assertEquals($expected, $this->apiController->getThumbnail(10, 10, 'unknown.jpg'));
}
public function testGetThumbnail(): void {
public function testGetThumbnailSharedNoDownload(): void {
$attributes = $this->createMock(IAttributes::class);
$attributes->expects(self::once())
->method('getAttribute')
->with('permissions', 'download')
->willReturn(false);
$share = $this->createMock(IShare::class);
$share->expects(self::once())
->method('getAttributes')
->willReturn($attributes);
$storage = $this->createMock(ISharedStorage::class);
$storage->expects(self::once())
->method('instanceOfStorage')
->with(ISharedStorage::class)
->willReturn(true);
$storage->expects(self::once())
->method('getShare')
->willReturn($share);
$file = $this->createMock(File::class);
$file->method('getId')->willReturn(123);
$file->method('getStorage')->willReturn($storage);
$this->userFolder->method('get')
->with('unknown.jpg')
->willReturn($file);
$this->preview->expects($this->never())
->method('getPreview');
$expected = new DataResponse(['message' => 'File not found.'], Http::STATUS_NOT_FOUND);
$this->assertEquals($expected, $this->apiController->getThumbnail(10, 10, 'unknown.jpg'));
}
public function testGetThumbnailShared(): void {
$share = $this->createMock(IShare::class);
$share->expects(self::once())
->method('getAttributes')
->willReturn(null);
$storage = $this->createMock(ISharedStorage::class);
$storage->expects(self::once())
->method('instanceOfStorage')
->with(ISharedStorage::class)
->willReturn(true);
$storage->expects(self::once())
->method('getShare')
->willReturn($share);
$file = $this->createMock(File::class);
$file->method('getId')->willReturn(123);
$file->method('getStorage')->willReturn($storage);
$this->userFolder->method('get')
->with($this->equalTo('known.jpg'))
->willReturn($file);
$preview = $this->createMock(ISimpleFile::class);
$preview->method('getName')->willReturn('my name');
$preview->method('getMTime')->willReturn(42);
$this->preview->expects($this->once())
->method('getPreview')
->with($this->equalTo($file), 10, 10, true)
->willReturn($preview);
$ret = $this->apiController->getThumbnail(10, 10, 'known.jpg');
$this->assertEquals(Http::STATUS_OK, $ret->getStatus());
$this->assertInstanceOf(FileDisplayResponse::class, $ret);
}
public function testGetThumbnail(): void {
$storage = $this->createMock(IStorage::class);
$storage->method('instanceOfStorage')->with(ISharedStorage::class)->willReturn(false);
$file = $this->createMock(File::class);
$file->method('getId')->willReturn(123);
$file->method('getStorage')->willReturn($storage);
$this->userFolder->method('get')
->with($this->equalTo('known.jpg'))
->willReturn($file);