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 |
Reporter | wally68 | Assigned To | dregad | ||
Priority | normal | Severity | major | Reproducibility | sometimes |
Status | closed | Resolution | fixed | ||
Product Version | 2.2.1 | ||||
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 | ||||
Description | 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. kind regards | ||||
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? |
|
line 306:
EDIT (dregad) fix markdown |
|
https://github.com/wuttke/mantisbt/commit/acfc7d444a016b440bd861da9cdb31c0be2a3e64 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? |
|
@vboctoradmin |
|
@wuttke what do you think? |
|
@vboctor 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 |
|
Alternate PR https://github.com/mantisbt/mantisbt/pull/1541 to the one proposed in 0022898:0058873 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 13:21 Details 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 bugnote's date. 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. Fixes 0022898 |
Affected Issues 0022898 |
|
mod - core/email_api.php | Diff File |