Don't allow anchors and queries in remote urls

Signed-off-by: Joas Schilling <coding@schilljs.com>
pull/19907/head
Joas Schilling 2020-03-12 13:43:29 +07:00
parent a5b0f41a9f
commit 3930ab8e8a
No known key found for this signature in database
GPG Key ID: 7076EA9751AACDDA
2 changed files with 50 additions and 30 deletions

@ -130,6 +130,9 @@ class ExternalSharesController extends Controller {
* @return DataResponse
*/
public function testRemote($remote) {
if (strpos($remote, '#') !== false || strpos($remote, '?') !== false) {
return new DataResponse(false);
}
if (
$this->testUrl('https://' . $remote . '/ocs-provider/') ||
$this->testUrl('https://' . $remote . '/ocs-provider/index.php') ||

@ -29,6 +29,9 @@ use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\JSONResponse;
use OCP\Http\Client\IClientService;
use OCP\IRequest;
use OCP\Http\Client\IResponse;
use OCP\Http\Client\IClient;
use OCA\Files_Sharing\External\Manager;
/**
* Class ExternalShareControllerTest
@ -45,12 +48,9 @@ class ExternalShareControllerTest extends \Test\TestCase {
protected function setUp(): void {
parent::setUp();
$this->request = $this->getMockBuilder('\\OCP\\IRequest')
->disableOriginalConstructor()->getMock();
$this->externalManager = $this->getMockBuilder('\\OCA\\Files_Sharing\\External\\Manager')
->disableOriginalConstructor()->getMock();
$this->clientService = $this->getMockBuilder('\\OCP\Http\\Client\\IClientService')
->disableOriginalConstructor()->getMock();
$this->request = $this->createMock(IRequest::class);
$this->externalManager = $this->createMock(Manager::class);
$this->clientService = $this->createMock(IClientService::class);
}
/**
@ -69,7 +69,7 @@ class ExternalShareControllerTest extends \Test\TestCase {
$this->externalManager
->expects($this->once())
->method('getOpenShares')
->will($this->returnValue(['MyDummyArray']));
->willReturn(['MyDummyArray']);
$this->assertEquals(new JSONResponse(['MyDummyArray']), $this->getExternalShareController()->index());
}
@ -93,35 +93,34 @@ class ExternalShareControllerTest extends \Test\TestCase {
}
public function testRemoteWithValidHttps() {
$client = $this->getMockBuilder('\\OCP\\Http\\Client\\IClient')
->disableOriginalConstructor()->getMock();
$response = $this->getMockBuilder('\\OCP\\Http\\Client\\IResponse')
->disableOriginalConstructor()->getMock();
$client = $this->createMock(IClient::class);
$response = $this->createMock(IResponse::class);
$response
->expects($this->exactly(2))
->method('getBody')
->will($this->onConsecutiveCalls('Certainly not a JSON string', '{"installed":true,"maintenance":false,"version":"8.1.0.8","versionstring":"8.1.0","edition":""}'));
->willReturnOnConsecutiveCalls(
['Certainly not a JSON string'],
['{"installed":true,"maintenance":false,"version":"8.1.0.8","versionstring":"8.1.0","edition":""}']
);
$client
->expects($this->any())
->method('get')
->will($this->returnValue($response));
->willReturn($response);
$this->clientService
->expects($this->exactly(2))
->method('newClient')
->will($this->returnValue($client));
->willReturn($client);
$this->assertEquals(new DataResponse('https'), $this->getExternalShareController()->testRemote('owncloud.org'));
$this->assertEquals(new DataResponse('https'), $this->getExternalShareController()->testRemote('nextcloud.com'));
}
public function testRemoteWithWorkingHttp() {
$client = $this->getMockBuilder('\\OCP\\Http\\Client\\IClient')
->disableOriginalConstructor()->getMock();
$response = $this->getMockBuilder('\\OCP\\Http\\Client\\IResponse')
->disableOriginalConstructor()->getMock();
$client = $this->createMock(IClient::class);
$response = $this->createMock(IResponse::class);
$client
->method('get')
->will($this->returnValue($response));
->willReturn($response);
$response
->expects($this->exactly(5))
->method('getBody')
@ -129,28 +128,46 @@ class ExternalShareControllerTest extends \Test\TestCase {
$this->clientService
->expects($this->exactly(5))
->method('newClient')
->will($this->returnValue($client));
->willReturn($client);
$this->assertEquals(new DataResponse('http'), $this->getExternalShareController()->testRemote('owncloud.org'));
$this->assertEquals(new DataResponse('http'), $this->getExternalShareController()->testRemote('nextcloud.com'));
}
public function testRemoteWithInvalidRemote() {
$client = $this->getMockBuilder('\\OCP\\Http\\Client\\IClient')
->disableOriginalConstructor()->getMock();
$response = $this->getMockBuilder('\\OCP\\Http\\Client\\IResponse')
->disableOriginalConstructor()->getMock();
$client = $this->createMock(IClient::class);
$response = $this->createMock(IResponse::class);
$client
->expects($this->exactly(6))
->method('get')
->will($this->returnValue($response));
->willReturn($response);
$response
->expects($this->exactly(6))
->method('getBody')
->will($this->returnValue('Certainly not a JSON string'));
->willReturn('Certainly not a JSON string');
$this->clientService
->expects($this->exactly(6))
->method('newClient')
->will($this->returnValue($client));
->willReturn($client);
$this->assertEquals(new DataResponse(false), $this->getExternalShareController()->testRemote('nextcloud.com'));
}
public function dataRemoteWithInvalidRemoteURLs(): array {
return [
['nextcloud.com?query'],
['nextcloud.com/#anchor'],
];
}
/**
* @dataProvider dataRemoteWithInvalidRemoteURLs
* @param string $remote
*/
public function testRemoteWithInvalidRemoteURLs(string $remote) {
$this->clientService
->expects($this->never())
->method('newClient');
$this->assertEquals(new DataResponse(false), $this->getExternalShareController()->testRemote('owncloud.org'));
$this->assertEquals(new DataResponse(false), $this->getExternalShareController()->testRemote($remote));
}
}