fix: use empty() to check if header exists

apps/files_sharing/lib/Controller/Share20OcsController.php

@@ -287,6 +287,7 @@ protected function formatShare(IShare $share, $received = false) {
 	 * Get a specific share by id
 	 *
 	 * @NoAdminRequired
+	 * @NoCSRFRequired
 	 *
 	 * @param string $id
 	 * @return Result
@@ -684,6 +685,7 @@ private function getSharedWithMe($node, $includeTags, $requestedShareTypes, $sta
 	 * the function will return an empty list.
 	 *
 	 * @NoAdminRequired
+	 * @NoCSRFRequired
 	 *
 	 * - Get shares by the current user
 	 * - Get shares by the current user and reshares (?reshares=true)

changelog/unreleased/40856

@@ -0,0 +1,6 @@
+Bugfix: Request header can hold an empty string if not set
+
+Due to Apache rewrite rules originally not existing headers can hold an empty
+string.
+
+https://github.com/owncloud/core/pull/40856

core/Controller/OcsController.php

@@ -110,6 +110,7 @@ public function checkPerson($login, $password) {
 	 * test: curl http://login:passwd@oc/core/ocs/v1.php/privatedata/getattribute
 	 *
 	 * @NoAdminRequired
+	 * @NoCSRFRequired
 	 *
 	 * @return Result
 	 */

lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php

@@ -142,7 +142,8 @@ public function beforeController($controller, $methodName) {
 		// CSRF check - also registers the CSRF token since the session may be closed later
 		Util::callRegister();
 		if (!$this->reflector->hasAnnotation('NoCSRFRequired')) {
-			if (!$this->request->passesCSRFCheck() && $this->request->getHeader("Authorization") === null) {
+			$hasNoAuthHeader = ($this->request->getHeader("Authorization") === null || trim($this->request->getHeader("Authorization")) === '');
+			if (!$this->request->passesCSRFCheck() && $hasNoAuthHeader) {
 				throw new CrossSiteRequestForgeryException();
 			}
 		}

tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php

@@ -40,6 +40,7 @@
 use OCP\ISession;
 use OCP\AppFramework\Controller;
 use OCP\IUserSession;
+use ReflectionException;
 use Test\TestCase;
 use OCP\AppFramework\Http\Response;
 use OCP\IConfig;
@@ -135,7 +136,7 @@ private function getMiddleware($isLoggedIn, $isAdminUser) {
 	 * @PublicPage
 	 * @NoCSRFRequired
 	 * @throws SecurityException
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testSetNavigationEntry() {
 		$this->navigationManager->expects($this->once())
@@ -150,7 +151,7 @@ public function testSetNavigationEntry() {
 	 * @param string $method
 	 * @param string $test
 	 * @param $status
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	private function ajaxExceptionStatus($method, $test, $status) {
 		$isLoggedIn = false;
@@ -178,7 +179,7 @@ private function ajaxExceptionStatus($method, $test, $status) {
 	}
 
 	/**
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testAjaxStatusLoggedInCheck() {
 		$this->ajaxExceptionStatus(
@@ -190,7 +191,7 @@ public function testAjaxStatusLoggedInCheck() {
 
 	/**
 	 * @NoCSRFRequired
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testAjaxNotAdminCheck() {
 		$this->ajaxExceptionStatus(
@@ -202,7 +203,7 @@ public function testAjaxNotAdminCheck() {
 
 	/**
 	 * @PublicPage
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testAjaxStatusCSRFCheck() {
 		$this->ajaxExceptionStatus(
@@ -215,10 +216,7 @@ public function testAjaxStatusCSRFCheck() {
 	/**
 	 * @PublicPage
 	 * @NoCSRFRequired
-	 * @throws \ReflectionException
-	 * @throws \ReflectionException
-	 * @throws \ReflectionException
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testAjaxStatusAllGood() {
 		$this->ajaxExceptionStatus(
@@ -247,7 +245,7 @@ public function testAjaxStatusAllGood() {
 	 * @PublicPage
 	 * @NoCSRFRequired
 	 * @throws SecurityException
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testNoChecks() {
 		$this->request->expects($this->never())
@@ -265,7 +263,7 @@ public function testNoChecks() {
 	 * @param string $expects
 	 * @param bool $shouldFail
 	 * @throws SecurityException
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	private function securityCheck($method, $expects, $shouldFail=false) {
 		// admin check requires login
@@ -292,10 +290,10 @@ private function securityCheck($method, $expects, $shouldFail=false) {
 	/**
 	 * @PublicPage
 	 * @throws SecurityException
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testCsrfCheck() {
-		$this->expectException(\OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException::class);
+		$this->expectException(CrossSiteRequestForgeryException::class);
 
 		$this->request->expects($this->once())
 			->method('passesCSRFCheck')
@@ -309,7 +307,7 @@ public function testCsrfCheck() {
 	 * @PublicPage
 	 * @NoCSRFRequired
 	 * @throws SecurityException
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testNoCsrfCheck() {
 		$this->request->expects($this->never())
@@ -323,7 +321,7 @@ public function testNoCsrfCheck() {
 	/**
 	 * @PublicPage
 	 * @throws SecurityException
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testFailCsrfCheck() {
 		$this->request->expects($this->once())
@@ -334,11 +332,29 @@ public function testFailCsrfCheck() {
 		$this->middleware->beforeController(__CLASS__, __FUNCTION__);
 	}
 
+	/**
+	 * @PublicPage
+	 * @throws SecurityException
+	 * @throws ReflectionException
+	 */
+	public function testFailCsrfCheckWithoutAuthHeader(): void {
+		$this->expectException(CrossSiteRequestForgeryException::class);
+		$this->request->expects($this->once())
+			->method('passesCSRFCheck')
+			->willReturn(false);
+		$this->request
+			->method('getHeader')
+			->willReturn('');
+
+		$this->reader->reflect(__CLASS__, __FUNCTION__);
+		$this->middleware->beforeController(__CLASS__, __FUNCTION__);
+	}
+
 	/**
 	 * @NoCSRFRequired
 	 * @NoAdminRequired
 	 * @throws SecurityException
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testLoggedInCheck() {
 		$this->securityCheck(__FUNCTION__, 'isLoggedIn');
@@ -348,7 +364,7 @@ public function testLoggedInCheck() {
 	 * @NoCSRFRequired
 	 * @NoAdminRequired
 	 * @throws SecurityException
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testFailLoggedInCheck() {
 		$this->securityCheck(__FUNCTION__, 'isLoggedIn', true);
@@ -357,7 +373,7 @@ public function testFailLoggedInCheck() {
 	/**
 	 * @NoCSRFRequired
 	 * @throws SecurityException
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testIsAdminCheck() {
 		$this->securityCheck(__FUNCTION__, 'isAdminUser');
@@ -366,7 +382,7 @@ public function testIsAdminCheck() {
 	/**
 	 * @NoCSRFRequired
 	 * @throws SecurityException
-	 * @throws \ReflectionException
+	 * @throws ReflectionException
 	 */
 	public function testFailIsAdminCheck() {
 		$this->securityCheck(__FUNCTION__, 'isAdminUser', true);