Skip to content

Commit 797b584

Browse files
Merge pull request #8578 from nextcloud/backport/8459/stable2.2
[stable2.2] fix: Check strict cookies for image proxy
2 parents fdcd70c + dc7d085 commit 797b584

File tree

2 files changed

+41
-2
lines changed

2 files changed

+41
-2
lines changed

lib/Controller/ProxyController.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
use OCP\IURLGenerator;
3737
use Psr\Http\Client\ClientExceptionInterface;
3838
use Psr\Log\LoggerInterface;
39+
use function file_get_contents;
3940

4041
class ProxyController extends Controller {
4142
private IURLGenerator $urlGenerator;
@@ -105,6 +106,12 @@ public function proxy(string $src): ProxyDownloadResponse {
105106
// close the session to allow parallel downloads
106107
$this->session->close();
107108

109+
// If strict cookies are set it means we come from the same domain so no open redirect
110+
if (!$this->request->passesStrictCookieCheck()) {
111+
$content = file_get_contents(__DIR__ . '/../../img/blocked-image.png');
112+
return new ProxyDownloadResponse($content, $src, 'application/octet-stream');
113+
}
114+
108115
$client = $this->clientService->newClient();
109116
try {
110117
$response = $client->get($src);

tests/Unit/Controller/ProxyControllerTest.php

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
/**
46
* @author Christoph Wurst <christoph@winzerhof-wurst.at>
57
*
@@ -156,11 +158,41 @@ public function testRedirectInvalidUrl() {
156158
$this->controller->redirect('ftps://example.com');
157159
}
158160

159-
public function testProxy() {
161+
public function testProxyWithoutCookies(): void {
160162
$src = 'http://example.com';
161-
$httpResponse = $this->createMock(IResponse::class);
162163
$content = '🐵🐵🐵';
164+
$this->session->expects($this->once())
165+
->method('close');
166+
$client = $this->getMockBuilder(IClient::class)->getMock();
167+
$this->clientService->expects(self::never())
168+
->method('newClient')
169+
->willReturn($client);
170+
$unexpected = new ProxyDownloadResponse(
171+
$content,
172+
$src,
173+
'application/octet-stream'
174+
);
175+
$this->controller = new ProxyController(
176+
$this->appName,
177+
$this->request,
178+
$this->urlGenerator,
179+
$this->session,
180+
$this->clientService,
181+
$this->logger
182+
);
183+
184+
$response = $this->controller->proxy($src);
163185

186+
$this->assertNotEquals($unexpected, $response);
187+
}
188+
189+
public function testProxy(): void {
190+
$src = 'http://example.com';
191+
$httpResponse = $this->createMock(IResponse::class);
192+
$content = '🐵🐵🐵';
193+
$this->request->expects(self::once())
194+
->method('passesStrictCookieCheck')
195+
->willReturn(true);
164196
$this->session->expects($this->once())
165197
->method('close');
166198
$client = $this->getMockBuilder(IClient::class)->getMock();

0 commit comments

Comments
 (0)