View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0022898||mantisbt||security||public||2017-05-18 06:51||2019-08-25 12:36|
|Target Version||2.22.0||Fixed in Version||2.22.0|
|Summary||0022898: Email for a new private bugnote was send to a non authorized reporter|
We have found a strange problem with private bug-notes:
After some code digging, we found this in core/email_api.php::email_collect_recipients, around line 461:
The recipient for a bugnote email is excluded if the bug date is equal to the bugnote date and the access level is wrong. You use the lastmod-timestamp from the bug and the bugnote to differ between a bug email and a bugnote email.
The timestamp for a bugnote is created by db_now() in core/bugnote_api.php::bugnote_add. The timestamp for the bug is created by db_now() in core/bug_api.php::bug_update_date. The function bug_update_date is called from bugnote_add.
In our opinion there is a potential gap to create two different timestamps for the bugnote and the bug especially on slow machines.
As a possible solution, function core/bug_api.php::bug_update_date may be extended with a default parameter $p_last_modified = 0 and the call from bugnote_add would set the timestamp from the bugnote as a parameter to bug_update_date.
|Tags||No tags attached.|
Instead of testing the bug timestamp against the bugnote timestamp in <b>core/email_api.php::email_collect_recipients</b> the function parameter <b>$p_notify_type</b> could be tested against 'bugnote' right?
EDIT (dregad) fix markdown
we'll check and then I'll create a pull request
Do we really need to timestamp check? Should we base this on the fact that the change is about a bugnote addition and having a bugnote id?
@wuttke what do you think?
We just stumbled across it in version 2.18.1.:
Am I correct that this problem is the same as in this bug here?
Well we never got a response to my review comment in the PR, nor to vboctor's question below, so you should really be asking @wuttke...
Unfortunately it seems that @wuttke is no longer active around here. It would be a shame if this issue just kept sitting here without any change because of that.
Is there any way for you mantis developers to implement a fix for this? At least for my organisation it would be a tremendous help, since we want to rely on our private bug notes to remain private ;)
@Herr.mullepulle you're right.
The question is, whether the proposed fix based on adjusting the timestamp to match is the proper solution (in which case it's a simple matter of adjusting @wuttke's pull request based on my review, which could be done easily), or if we should implement the alternative, seemingly more robust approach suggested by @wally68 and @vboctor to rely on notification type.
Confirming the issue.
To reproduce: the race condition can be forced by adding a
I think it might still be a good thing to ensure the bugnote's timestamp is effectively always equal to that of the parent bug's last_updated timestamp
MantisBT: master ad42c3ca
2019-08-10 17:21:41Details Diff
|Prevent email about private note to unprivileged users
In email_collect_recipient(), the logic to exclude users who can't see
bugnotes relied on comparing the issue's last updated timestamp with the
Since these dates are not necessarily equal as they are updated
separately when a bugnote is added, this may result in a race condition
causing a notification e-mail about a new private bugnote to be sent to
users not authorized to see them.
Since email_collect_recipient()'s $p_bugnote_id parameter is always null
except for 'bugnote' notifications, the date check is not necessary; it
is sufficient to check that $p_bugnote_id is not null.
|mod - core/email_api.php||Diff File|
|2017-05-18 06:51||wally68||New Issue|
|2017-05-19 04:57||wally68||Note Added: 0056905|
|2017-10-17 10:04||atrol||Relationship added||has duplicate 0023492|
|2018-02-14 15:51||wuttke||Note Added: 0058870|
|2018-02-14 15:53||wuttke||Note Added: 0058871|
|2018-02-15 02:54||wuttke||Note Added: 0058873|
|2018-02-19 10:39||dregad||Note Edited: 0058870||View Revisions|
|2018-02-28 23:19||vboctoradmin||Note Added: 0059059|
|2018-03-01 02:23||wally68||Note Added: 0059060|
|2018-03-29 19:08||vboctor||Note Added: 0059356|
|2019-07-29 06:52||Herr.mullepulle||Note Added: 0062446|
|2019-07-29 11:52||dregad||Note Added: 0062447|
|2019-08-09 05:06||Herr.mullepulle||Note Added: 0062542|
|2019-08-09 10:01||dregad||Description Updated||View Revisions|
|2019-08-09 10:03||dregad||Description Updated||View Revisions|
|2019-08-09 10:45||dregad||Note Added: 0062545|
|2019-08-09 11:09||dregad||Status||new => confirmed|
|2019-08-09 11:09||dregad||Note Added: 0062547|
|2019-08-09 20:19||dregad||Assigned To||=> dregad|
|2019-08-09 20:19||dregad||Severity||minor => major|
|2019-08-09 20:19||dregad||Status||confirmed => assigned|
|2019-08-09 20:19||dregad||Category||email => security|
|2019-08-09 20:19||dregad||Target Version||=> 2.22.0|
|2019-08-09 20:21||dregad||Note Added: 0062551|
|2019-08-09 20:40||dregad||Note Edited: 0062551||View Revisions|
|2019-08-09 20:41||dregad||Note Edited: 0062551||View Revisions|
|2019-08-24 22:23||dregad||Changeset attached||=> MantisBT master ad42c3ca|
|2019-08-24 22:23||dregad||Status||assigned => resolved|
|2019-08-24 22:23||dregad||Resolution||open => fixed|
|2019-08-24 22:23||dregad||Fixed in Version||=> 2.22.0|
|2019-08-25 12:36||vboctor||Status||resolved => closed|