View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0022898||mantisbt||security||public||2017-05-18 06:51||2019-08-09 20:41|
|Target Version||2.22.0||Fixed in Version|
|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
|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|