Apply hardenings found by automated scanner

js/notifications-main.mjs

@@ -1,3 +1,3 @@
-const __vite__mapDeps=(i,m=__vite__mapDeps,d=(m.f||(m.f=[window.OC.filePath('notifications', '', 'js/NotificationsApp-Cq_-j9nk.chunk.mjs'),window.OC.filePath('notifications', '', 'js/_plugin-vue_export-helper-DoVzk5Kw.chunk.mjs'),window.OC.filePath('notifications', '', 'js/style-DnTnUg81.chunk.mjs'),window.OC.filePath('notifications', '', 'css/style-DzIwnQsS.chunk.css'),window.OC.filePath('notifications', '', 'css/_plugin-vue_export-helper-CFp4Dbip.chunk.css'),window.OC.filePath('notifications', '', 'js/vite-preload-helper-DxYC2qmj.chunk.mjs'),window.OC.filePath('notifications', '', 'js/BrowserStorage-j57i456t.chunk.mjs'),window.OC.filePath('notifications', '', 'css/NotificationsApp-Bhbv8ELa.chunk.css')])))=>i.map(i=>d[i]);
-import{_ as o}from"./vite-preload-helper-DxYC2qmj.chunk.mjs";import{c as i,d as n}from"./style-DnTnUg81.chunk.mjs";const m=n(()=>o(()=>import("./NotificationsApp-Cq_-j9nk.chunk.mjs").then(t=>t.g),__vite__mapDeps([0,1,2,3,4,5,6,7]),import.meta.url));i(m).mount("#notifications");
+const __vite__mapDeps=(i,m=__vite__mapDeps,d=(m.f||(m.f=[window.OC.filePath('notifications', '', 'js/NotificationsApp-D5yEkr8D.chunk.mjs'),window.OC.filePath('notifications', '', 'js/_plugin-vue_export-helper-DoVzk5Kw.chunk.mjs'),window.OC.filePath('notifications', '', 'js/style-DnTnUg81.chunk.mjs'),window.OC.filePath('notifications', '', 'css/style-DzIwnQsS.chunk.css'),window.OC.filePath('notifications', '', 'css/_plugin-vue_export-helper-CFp4Dbip.chunk.css'),window.OC.filePath('notifications', '', 'js/vite-preload-helper-DxYC2qmj.chunk.mjs'),window.OC.filePath('notifications', '', 'js/BrowserStorage-j57i456t.chunk.mjs'),window.OC.filePath('notifications', '', 'css/NotificationsApp-BJcCFRZ6.chunk.css')])))=>i.map(i=>d[i]);
+import{_ as o}from"./vite-preload-helper-DxYC2qmj.chunk.mjs";import{c as i,d as n}from"./style-DnTnUg81.chunk.mjs";const m=n(()=>o(()=>import("./NotificationsApp-D5yEkr8D.chunk.mjs").then(t=>t.g),__vite__mapDeps([0,1,2,3,4,5,6,7]),import.meta.url));i(m).mount("#notifications");
 //# sourceMappingURL=notifications-main.mjs.map

js/service-worker.js

@@ -91,11 +91,27 @@ self.addEventListener('push', function(event) {
 	}
 })
 
+/**
+ * @param {string|undefined} link absolute or relative link
+ * @return {string|null} the absolute link when it points to this instance's origin
+ */
+function sameOriginUrl(link) {
+	if (!link) {
+		return null
+	}
+	try {
+		const url = new URL(link, self.location.origin)
+		return url.origin === self.location.origin ? url.href : null
+	} catch {
+		return null
+	}
+}
+
 self.addEventListener('notificationclick', function(event) {
 	event.notification.close()
-	const link = event.notification.data && event.notification.data.link
+	const link = sameOriginUrl(event.notification.data && event.notification.data.link)
 	if (!link) {
-		// no target: focus an existing window of this instance if any
+		// no or foreign target: focus an existing window of this instance if any
 		event.waitUntil(self.clients.matchAll({ type: 'window' }).then((clientList) => {
 			for (const client of clientList) {
 				if (client.url.startsWith(self.location.origin)) {

lib/Controller/PushController.php

@@ -16,6 +16,7 @@
 use OCP\AppFramework\Http\Attribute\ApiRoute;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
+use OCP\AppFramework\Http\Attribute\UserRateLimit;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
 use OCP\Authentication\Exceptions\InvalidTokenException;
@@ -60,8 +61,9 @@ public function __construct(
 	 * 401: Missing permissions to register device
 	 */
 	#[NoAdminRequired]
+	#[UserRateLimit(limit: 10, period: 3600)]
 	#[ApiRoute(verb: 'POST', url: '/api/{apiVersion}/push', requirements: ['apiVersion' => '(v2)'])]
-	public function registerDevice(string $pushTokenHash, string $devicePublicKey, string $proxyServer): DataResponse {
+	public function registerDevice(#[\SensitiveParameter] string $pushTokenHash, string $devicePublicKey, string $proxyServer): DataResponse {
 		$user = $this->userSession->getUser();
 		if (!$user instanceof IUser) {
 			return new DataResponse([], Http::STATUS_UNAUTHORIZED);
@@ -179,7 +181,7 @@ public function removeDevice(): DataResponse {
 	/**
 	 * @return bool If the hash was new to the database
 	 */
-	protected function savePushToken(IUser $user, IToken $token, string $deviceIdentifier, string $devicePublicKey, string $pushTokenHash, string $proxyServer, string $appType): bool {
+	protected function savePushToken(IUser $user, IToken $token, string $deviceIdentifier, string $devicePublicKey, #[\SensitiveParameter] string $pushTokenHash, string $proxyServer, string $appType): bool {
 		$query = $this->db->getQueryBuilder();
 		$query->select('*')
 			->from('notifications_pushhash')
@@ -202,7 +204,7 @@ protected function savePushToken(IUser $user, IToken $token, string $deviceIdent
 	/**
 	 * @return bool If the entry was created
 	 */
-	protected function insertPushToken(IUser $user, IToken $token, string $deviceIdentifier, string $devicePublicKey, string $pushTokenHash, string $proxyServer, string $appType): bool {
+	protected function insertPushToken(IUser $user, IToken $token, string $deviceIdentifier, string $devicePublicKey, #[\SensitiveParameter] string $pushTokenHash, string $proxyServer, string $appType): bool {
 		$devicePublicKeyHash = hash('sha512', $devicePublicKey);
 
 		$query = $this->db->getQueryBuilder();
@@ -223,7 +225,7 @@ protected function insertPushToken(IUser $user, IToken $token, string $deviceIde
 	/**
 	 * @return bool If the entry was updated
 	 */
-	protected function updatePushToken(IUser $user, IToken $token, string $devicePublicKey, string $pushTokenHash, string $proxyServer, string $appType): bool {
+	protected function updatePushToken(IUser $user, IToken $token, string $devicePublicKey, #[\SensitiveParameter] string $pushTokenHash, string $proxyServer, string $appType): bool {
 		$devicePublicKeyHash = hash('sha512', $devicePublicKey);
 
 		$query = $this->db->getQueryBuilder();
@@ -254,7 +256,7 @@ protected function deletePushToken(IUser $user, IToken $token): bool {
 	/**
 	 * @return bool If the entry was deleted
 	 */
-	protected function deletePushTokenByHash(IUser $user, string $pushTokenHash): bool {
+	protected function deletePushTokenByHash(IUser $user, #[\SensitiveParameter] string $pushTokenHash): bool {
 		$query = $this->db->getQueryBuilder();
 		$query->delete('notifications_pushhash')
 			->where($query->expr()->eq('uid', $query->createNamedParameter($user->getUID())))

lib/Controller/WebPushController.php

@@ -18,6 +18,7 @@
 use OCP\AppFramework\Http\Attribute\ApiRoute;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\Attribute\OpenAPI;
+use OCP\AppFramework\Http\Attribute\UserRateLimit;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
 use OCP\AppFramework\Services\IAppConfig;
@@ -81,8 +82,9 @@ public function getVapid(): DataResponse {
 	 * 403: Web push is disabled by the administrator
 	 */
 	#[NoAdminRequired]
+	#[UserRateLimit(limit: 20, period: 600)]
 	#[ApiRoute(verb: 'POST', url: '/api/{apiVersion}/webpush', requirements: ['apiVersion' => '(v2)'])]
-	public function registerWP(string $endpoint, string $uaPublicKey, string $auth, string $appTypes): DataResponse {
+	public function registerWP(#[\SensitiveParameter] string $endpoint, string $uaPublicKey, #[\SensitiveParameter] string $auth, string $appTypes): DataResponse {
 		if (!$this->appConfig->getAppValueBool('webpush_enabled')) {
 			return new DataResponse(['message' => 'WEBPUSH_DISABLED'], Http::STATUS_FORBIDDEN);
 		}
@@ -162,7 +164,7 @@ public function registerWP(string $endpoint, string $uaPublicKey, string $auth,
 	 */
 	#[NoAdminRequired]
 	#[ApiRoute(verb: 'POST', url: '/api/{apiVersion}/webpush/activate', requirements: ['apiVersion' => '(v2)'])]
-	public function activateWP(string $activationToken): DataResponse {
+	public function activateWP(#[\SensitiveParameter] string $activationToken): DataResponse {
 		if (!$this->appConfig->getAppValueBool('webpush_enabled')) {
 			return new DataResponse(['message' => 'WEBPUSH_DISABLED'], Http::STATUS_FORBIDDEN);
 		}
@@ -245,7 +247,7 @@ protected function getWPClient(): WebPushClient {
 	 * - CREATED if the user didn't have an activated subscription with this endpoint, pubkey and auth
 	 * - UPDATED if the subscription has been updated (use to change appTypes)
 	 */
-	protected function saveSubscription(IUser $user, IToken|ISession $token, string $endpoint, string $uaPublicKey, string $auth, string $appTypes): array {
+	protected function saveSubscription(IUser $user, IToken|ISession $token, #[\SensitiveParameter] string $endpoint, string $uaPublicKey, #[\SensitiveParameter] string $auth, string $appTypes): array {
 		$query = $this->db->getQueryBuilder();
 		$query->select('*')
 			->from('notifications_webpush')
@@ -283,7 +285,7 @@ protected function saveSubscription(IUser $user, IToken|ISession $token, string
 	 * - NO_TOKEN if we don't have this token
 	 * - NO_SUB if we don't have this subscription
 	 */
-	protected function activateSubscription(IUser $user, IToken|ISession $token, string $activationToken): ActivationSubStatus {
+	protected function activateSubscription(IUser $user, IToken|ISession $token, #[\SensitiveParameter] string $activationToken): ActivationSubStatus {
 		$query = $this->db->getQueryBuilder();
 		$query->select('*')
 			->from('notifications_webpush')
@@ -317,7 +319,7 @@ protected function activateSubscription(IUser $user, IToken|ISession $token, str
 	 * @param string $appTypes comma separated list of types
 	 * @return bool If the entry was created
 	 */
-	protected function insertSubscription(IUser $user, IToken|ISession $token, string $endpoint, string $uaPublicKey, string $auth, string $activationToken, string $appTypes): bool {
+	protected function insertSubscription(IUser $user, IToken|ISession $token, #[\SensitiveParameter] string $endpoint, string $uaPublicKey, #[\SensitiveParameter] string $auth, #[\SensitiveParameter] string $activationToken, string $appTypes): bool {
 		$query = $this->db->getQueryBuilder();
 		$query->insert('notifications_webpush')
 			->values([
@@ -336,7 +338,7 @@ protected function insertSubscription(IUser $user, IToken|ISession $token, strin
 	 * @param string $appTypes comma separated list of types
 	 * @return bool If the entry was updated
 	 */
-	protected function updateSubscription(IUser $user, IToken|ISession $token, string $endpoint, string $uaPublicKey, string $auth, string $appTypes): bool {
+	protected function updateSubscription(IUser $user, IToken|ISession $token, #[\SensitiveParameter] string $endpoint, string $uaPublicKey, #[\SensitiveParameter] string $auth, string $appTypes): bool {
 		$query = $this->db->getQueryBuilder();
 		$query->update('notifications_webpush')
 			->set('endpoint', $query->createNamedParameter($endpoint))

lib/Push.php

@@ -479,6 +479,8 @@ public function proxyPushToDevice(int $id, IUser $user, array $devices, INotific
 			} catch (\InvalidArgumentException) {
 				// Failed to encrypt message for device: public key is invalid
 				$this->deleteProxyPushToken($device['token']);
+			} catch (\RuntimeException) {
+				// Failed to sign message, already logged
 			}
 		}
 		$this->printInfo('');
@@ -668,6 +670,8 @@ public function proxyPushDeleteToDevice(string $userId, IUser $user, array $devi
 			} catch (\InvalidArgumentException) {
 				// Failed to encrypt message for device: public key is invalid
 				$this->deleteProxyPushToken($device['token']);
+			} catch (\RuntimeException) {
+				// Failed to sign message, already logged
 			}
 		}
 
@@ -992,6 +996,19 @@ protected function getNotifTopicAndUrgency(string $app, string $type): array {
 		return $res;
 	}
 
+	/**
+	 * @throws \RuntimeException when the data could not be signed
+	 */
+	protected function sign(string $data, #[\SensitiveParameter] string $userPrivateKey): string {
+		if (!openssl_sign($data, $signature, $userPrivateKey, OPENSSL_ALGO_SHA512)) {
+			$error = openssl_error_string() ?: 'Unknown OpenSSL error';
+			$this->log->error($error, ['app' => 'notifications']);
+			$this->printInfo('<error>Failed to sign encrypted push subject: "' . $error . '"</error>');
+			throw new \RuntimeException('Failed to sign message for device');
+		}
+		return $signature;
+	}
+
 	/**
 	 * @param string $userPrivateKey
 	 * @param array $device
@@ -1001,8 +1018,9 @@ protected function getNotifTopicAndUrgency(string $app, string $type): array {
 	 * @psalm-return array{deviceIdentifier: string, pushTokenHash: string, subject: string, signature: string, priority: string, type: string}
 	 * @throws InvalidTokenException
 	 * @throws \InvalidArgumentException
+	 * @throws \RuntimeException
 	 */
-	protected function encryptAndSign(string $userPrivateKey, array $device, int $id, INotification $notification): array {
+	protected function encryptAndSign(#[\SensitiveParameter] string $userPrivateKey, #[\SensitiveParameter] array $device, int $id, INotification $notification): array {
 		$data = $this->encodeNotif($id, $notification, 200);
 		$ret = $this->getNotifTopicAndUrgency($data['app'], $data['type']);
 		$priority = $ret['urgency'];
@@ -1013,19 +1031,15 @@ protected function encryptAndSign(string $userPrivateKey, array $device, int $id
 		$this->printInfo('Device public key size: ' . strlen((string)$device['devicepublickey']));
 		$this->printInfo('Data to encrypt is: ' . $jsonData);
 
-		$padding = $this->appConfig->getAppValueString('push_encryption_padding', 'OAEP') === 'OAEP' ? OPENSSL_PKCS1_OAEP_PADDING : OPENSSL_PKCS1_PADDING;
-		if (!openssl_public_encrypt($jsonData, $encryptedSubject, $device['devicepublickey'], $padding)) {
+		if (!openssl_public_encrypt($jsonData, $encryptedSubject, $device['devicepublickey'], OPENSSL_PKCS1_OAEP_PADDING)) {
 			$error = openssl_error_string() ?: 'Unknown OpenSSL error';
 			$this->log->error($error, ['app' => 'notifications']);
 			$this->printInfo('<error>Error while encrypting data: "' . $error . '"</error>');
 			throw new \InvalidArgumentException('Failed to encrypt message for device');
 		}
 
-		if (openssl_sign($encryptedSubject, $signature, $userPrivateKey, OPENSSL_ALGO_SHA512)) {
-			$this->printInfo('Signed encrypted push subject');
-		} else {
-			$this->printInfo('<error>Failed to signed encrypted push subject</error>');
-		}
+		$signature = $this->sign($encryptedSubject, $userPrivateKey);
+		$this->printInfo('Signed encrypted push subject');
 		$base64EncryptedSubject = base64_encode($encryptedSubject);
 		$base64Signature = base64_encode($signature);
 
@@ -1047,20 +1061,20 @@ protected function encryptAndSign(string $userPrivateKey, array $device, int $id
 	 * @psalm-return array{remaining: array<array-key, int>, payload: array{deviceIdentifier: string, pushTokenHash: string, subject: string, signature: string, priority: string, type: string}}
 	 * @throws InvalidTokenException
 	 * @throws \InvalidArgumentException
+	 * @throws \RuntimeException
 	 */
-	protected function encryptAndSignDelete(string $userPrivateKey, array $device, ?array $ids): array {
+	protected function encryptAndSignDelete(#[\SensitiveParameter] string $userPrivateKey, #[\SensitiveParameter] array $device, ?array $ids): array {
 		$ret = $this->encodeDeleteNotifs($ids);
 		$remainingIds = $ret['remaining'];
 		$data = $ret['data'];
 
-		$padding = $this->appConfig->getAppValueString('push_encryption_padding', 'OAEP') === 'OAEP' ? OPENSSL_PKCS1_OAEP_PADDING : OPENSSL_PKCS1_PADDING;
-		if (!openssl_public_encrypt(json_encode($data, JSON_THROW_ON_ERROR), $encryptedSubject, $device['devicepublickey'], $padding)) {
+		if (!openssl_public_encrypt(json_encode($data, JSON_THROW_ON_ERROR), $encryptedSubject, $device['devicepublickey'], OPENSSL_PKCS1_OAEP_PADDING)) {
 			$error = openssl_error_string() ?: 'Unknown OpenSSL error';
 			$this->log->error($error, ['app' => 'notifications']);
 			throw new \InvalidArgumentException('Failed to encrypt message for device');
 		}
 
-		openssl_sign($encryptedSubject, $signature, $userPrivateKey, OPENSSL_ALGO_SHA512);
+		$signature = $this->sign($encryptedSubject, $userPrivateKey);
 		$base64EncryptedSubject = base64_encode($encryptedSubject);
 		$base64Signature = base64_encode($signature);
 
@@ -1190,7 +1204,7 @@ protected function deleteWebPushToken(int $tokenId): bool {
 	 * @param string $endpoint
 	 * @return bool
 	 */
-	protected function deleteWebPushTokenByEndpoint(string $endpoint): bool {
+	protected function deleteWebPushTokenByEndpoint(#[\SensitiveParameter] string $endpoint): bool {
 		$query = $this->db->getQueryBuilder();
 		$query->delete('notifications_webpush')
 			->where($query->expr()->eq('endpoint', $query->createNamedParameter($endpoint)));

lib/WebPushClient.php

@@ -43,7 +43,7 @@ public static function isValidP256dh(string $key): bool {
 		return true;
 	}
 
-	public static function isValidAuth(string $auth): bool {
+	public static function isValidAuth(#[\SensitiveParameter] string $auth): bool {
 		if (!preg_match('/^[A-Za-z0-9_-]{22}=*$/', $auth)) {
 			return false;
 		}
@@ -118,7 +118,7 @@ public function getVapidPublicKey(): string {
 	/**
 	 * Send one notification - blocking (should be avoided most of the time)
 	 */
-	public function notify(string $endpoint, string $uaPublicKey, string $auth, string $body): void {
+	public function notify(#[\SensitiveParameter] string $endpoint, string $uaPublicKey, #[\SensitiveParameter] string $auth, #[\SensitiveParameter] string $body): void {
 		$c = $this->getClient();
 		$c->queueNotification(
 			new Subscription($endpoint, $uaPublicKey, $auth, 'aes128gcm'),
@@ -136,7 +136,7 @@ public function notify(string $endpoint, string $uaPublicKey, string $auth, stri
 	 * Queue one notification. [flush] needs to be called to actually send the notifications
 	 * @throws \ErrorException
 	 */
-	public function enqueue(string $endpoint, string $uaPublicKey, string $auth, string $body, string $urgency = 'normal', ?string $topic = null): void {
+	public function enqueue(#[\SensitiveParameter] string $endpoint, string $uaPublicKey, #[\SensitiveParameter] string $auth, #[\SensitiveParameter] string $body, string $urgency = 'normal', ?string $topic = null): void {
 		$c = $this->getClient();
 		$c->queueNotification(
 			new Subscription($endpoint, $uaPublicKey, $auth, 'aes128gcm'),

src/Components/ActionButton.vue

@@ -6,6 +6,7 @@
 <script setup>
 import { computed } from 'vue'
 import NcButton from '@nextcloud/vue/components/NcButton'
+import { isSafeLink } from '../utils/link.js'
 
 /**
  * @typedef {object} NotificationAction
@@ -25,7 +26,7 @@ const props = defineProps({
 
 const emit = defineEmits(['click'])
 
-const isWebLink = computed(() => props.action.type === 'WEB')
+const isWebLink = computed(() => props.action.type === 'WEB' && isSafeLink(props.action.link))
 
 /**
  * Emits a click event with the action details

src/Components/NotificationItem.vue

@@ -106,6 +106,7 @@ import ActionButton from './ActionButton.vue'
 import DefaultParameter from './Parameters/DefaultParameter.vue'
 import FileParameter from './Parameters/FileParameter.vue'
 import UserParameter from './Parameters/UserParameter.vue'
+import { isSafeLink } from '../utils/link.js'
 
 /**
  * @typedef {object} NotificationItem
@@ -174,7 +175,7 @@ export default {
 		},
 
 		useLink() {
-			if (!this.notification.link) {
+			if (!isSafeLink(this.notification.link)) {
 				return false
 			}
 

src/Components/Parameters/DefaultParameter.vue

@@ -10,7 +10,7 @@
 		<strong>{{ name }}</strong>
 	</a>
 	<a
-		v-else-if="link"
+		v-else-if="hasSafeLink"
 		:href="link"
 		class="external"
 		target="_blank"
@@ -21,6 +21,8 @@
 </template>
 
 <script>
+import { isSafeLink } from '../../utils/link.js'
+
 export default {
 	name: 'DefaultParameter',
 	props: {
@@ -47,8 +49,12 @@ export default {
 	},
 
 	computed: {
+		hasSafeLink() {
+			return isSafeLink(this.link)
+		},
+
 		hasInternalLink() {
-			return this.link && (
+			return this.hasSafeLink && (
 				this.type === 'deck-board'
 				|| this.type === 'deck-card'
 			)