Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification-Issue in NotificationService #361

Open
heikojentzsch opened this issue Mar 14, 2021 · 0 comments
Open

Notification-Issue in NotificationService #361

heikojentzsch opened this issue Mar 14, 2021 · 0 comments

Comments

@heikojentzsch
Copy link

heikojentzsch commented Mar 14, 2021

there is from my personal perpective a security issue in the Class NotificationService inside the Method "notifyForumSubscribers(...)"

the method iterates over the forum structure setting parent forum as current forum. doing this the readAccess is always checked against the current forum which is in this moment the parent forum. in line 139 i think there has to be a check against the current forum AND always against the forum in which the new topic is written.

We fixed it in our instance by editing the method to the following version where we added a variable "$originalforum" which is additionally checked:

	protected function notifyForumSubscribers(Forum $forum, Topic $topic, Post $post) {

		$subject = Localization::translate('Mail_Subscribe_NewTopic_Subject');
		$messageTemplate = Localization::translate('Mail_Subscribe_NewTopic_Body');
		$postAuthorUid = $post->getAuthor()->getUid();

		$notifiedSubscribers = [];

		$originalforum = $forum;

		while ($forum) {
			$message = $this->getMessage($forum, $topic, $post, $messageTemplate, $this->getForumUnsubscribeLink($forum));

			foreach ($forum->getSubscribers() as $subscriber) {
				if (!$notifiedSubscribers[$subscriber->getUid()] && $forum->checkReadAccess($subscriber) && $originalforum->checkReadAccess($subscriber) && $subscriber->getUid() !== $postAuthorUid) {
					$subscriberMessage = nl2br(str_replace('###RECIPIENT###', $subscriber->getUsername(), $message));
					$this->htmlMailingService->sendMail($subscriber, $subject, $subscriberMessage);
					$notifiedSubscribers[$subscriber->getUid()] = TRUE;
				}
			}

			$forum = $forum->getParent();
		}
	}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant