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

Jump to bottom

Limit notifications of not authenticated sessions #8307

Closed
SR-mkuhn opened this issue Jul 9, 2015 · 5 comments
Closed

Limit notifications of not authenticated sessions #8307

SR-mkuhn opened this issue Jul 9, 2015 · 5 comments
Assignees
@tsteur
Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github.
Milestone
2.15.0

Comments

@SR-mkuhn
Copy link

SR-mkuhn commented Jul 9, 2015

If an attacker invokes an error like #8268 (he doesn't need to be authenticated for that) , a notification will be saved in the data field of his session.
As those notifications are stored until they are displayed (but aren't), the data field grows over time.
This can be used to fill hdd space or database space (if database session handling is active).

If database replication (mysql) is activated binlog grows very quick too, and uses a lot of hdd space (all of it).

A solution could be a limitation of notifications based on loginstatus.
@tsteur
Copy link
Member

tsteur commented Jul 13, 2015

Limiting the number of notifications sounds like a good idea in general 👍

@mattab
Copy link
Member

mattab commented Jul 15, 2015

For security reasons it sounds like a very good idea to limit number of notifications. @tsteur do you have any suggestion how this could work?

@mattab mattab added this to the 2.15.0 milestone Jul 15, 2015
@mattab mattab added the c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. label Jul 15, 2015
@SR-mkuhn SR-mkuhn mentioned this issue Jul 16, 2015
Open
@mattab
Copy link
Member

mattab commented Jul 16, 2015

Also for performance reasons this issue is important, see #8308

@tsteur
Copy link
Member

tsteur commented Jul 16, 2015

I'm not sure how notifications are implemented right now (especially re logging). At least initially they were stored in an array in the session so we could simply limit the number of notifications by using array_splice or so.

@mnapoli
Copy link
Contributor

mnapoli commented Jul 19, 2015

especially re logging

FYI logging simply uses the notification system, so it works like the rest of the notifications (which I don't know how it works :p that was only my 2c).

@tsteur tsteur self-assigned this Jul 20, 2015
tsteur added a commit that referenced this issue Jul 20, 2015
@tsteur
refs #8307 limit the number of stored notifications to 30
5c6b388
@tsteur tsteur mentioned this issue Jul 20, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tsteur tsteur

Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github.
Projects
None yet
Milestone
2.15.0
Development

No branches or pull requests
4 participants
@tsteur @mattab @mnapoli @SR-mkuhn