Patch #27676

Information leak on roadmap and versions view

Added by Jan from Planio www.plan.io about 1 year ago. Updated 2 months ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:Roadmap
Target version:4.0.0

Description

When limiting a role's permission to only access "Issues created by or assigned to the user", the roadmap (/projects/:identifier/roadmap) and version details (versions/:id) view leaks information about inaccessible issues and time estimations. Due to missing permission checks in Version#fixed_issues the restricted user may see the overall number of issues, their status, tracker, author, category, and time estimations.

We think, this a security-relevant information leak and it should be fixed and announced responsibly. Attached you may find a proposed patch which includes tests and a fix.

The attached patch changes the Version model, so that the calculation methods (closed_issues_count, open_issues_count, etc) are now also available on the fixed_issues relation proxy object. In a second step, all relevant places, where those calcuation methods are used, are updated to include the visible scope. This fixes the roadmap view, the version details view and the version summary in the Gantt chart.

This bug was reported by a Planio user, the patch series was developed by Gregor Schmidt.

0004-Fixes-visibility-checks-for-version.fixed_issues-in-.patch Magnifier (3.11 KB) Jan from Planio www.plan.io, 2017-11-29 17:24

0002-Adds-visibility-checks-on-version-views.patch Magnifier (4.93 KB) Jan from Planio www.plan.io, 2017-11-29 17:24

0003-Performance-opt-cache-AR-Proxy-for-Version-fixed_iss.patch Magnifier (5.93 KB) Jan from Planio www.plan.io, 2017-11-29 17:24

0001-Moves-issue-calculations-into-the-fixed_issues-relat.patch Magnifier (7.17 KB) Jan from Planio www.plan.io, 2017-11-29 17:24


Related issues

Related to Redmine - Defect #15258: Roadmap Issue Count off Closed
Duplicated by Redmine - Defect #19187: Roadmap links in subproject Closed
Duplicated by Redmine - Defect #19059: Wrong number of issues for a version in the roadmap Closed

Associated revisions

Revision 17050
Added by Jean-Philippe Lang about 1 year ago

Moves issue calculations into the fixed_issues relation (#27676).

This way, we can reuse them on refined relations,
e.g. version.fixed_issues.closed_count vs. version.fixed_issues.visible.closed_count

Patch by Gregor Schmidt.

Revision 17051
Added by Jean-Philippe Lang about 1 year ago

Adds visibility checks on version views (#27676).

Previously not all data on the roadmap and version view where properly
checked against the issue visibility setting. Unprivileged users were
able to see the total number of issues, their estimations and the
open/close status - even if the user was only allowed to see their own issues.

Patch by Gregor Schmidt.

Revision 17052
Added by Jean-Philippe Lang about 1 year ago

Performance opt - cache AR Proxy for Version#fixed_issues.visible (#27676).

Patch by Gregor Schmidt.

Revision 17053
Added by Jean-Philippe Lang about 1 year ago

Fixes visibility checks for version.fixed_issues in Gantt (#27676).

Like the version page - the Gantt chart featured a "percent done" info
for each version, which wasn't properly limited to visible issues.

Patch by Gregor Schmidt.

Revision 17251
Added by Go MAEDA 11 months ago

Add a test to ensure that issue calculations on version details page take into account only visible issues (#27676).

Patch by Marius BALTEANU.

History

#1 Updated by Jean-Philippe Lang about 1 year ago

I've committed the patch serie, thanks.

This issue was already reported long time ago and it was chosen not to change the behaviour (see #15258). With this change, different users might now see different progress values for the same version and this can be confusing. I think we should add a message for when there are issues assigned to the version that are not visible to the user, for example:

  • When all issues are visible: no change
  • When there are no visible issues but other issues exist: "No visible issues for this version" (instead of "No issues for this version")
  • When there are visible issues and other issues exist: "Some issues assigned to this version are not visible and not taken into account" (message added)
  • When there are no issues: no change ("No issues for this version")

What do you think? IMO, it's important to let the user know that are other (not visible) issues that are assigned to the version.

#2 Updated by Jean-Philippe Lang about 1 year ago

#3 Updated by Jean-Philippe Lang about 1 year ago

Also reported in #9411 and #15248

#4 Updated by Jan from Planio www.plan.io about 1 year ago

Thank you for your feedback. Here's what Gregor said:

I agree. It may be confusing, that two users may see different roadmaps. On the other hand, the same is true for issue lists, Gantt charts and many other views. This would be the first place, where a special note about invisible elements is added. It feels like a paradigm shift to me.

I don't want to argue against that change. I merely want to be sure, that it's done without proper thought.

#5 Updated by Toshi MARUYAMA about 1 year ago

How about #19187 and #19059?
Marius provides test case in #19187#note-4.

#6 Updated by Go MAEDA 11 months ago

  • Target version set to 4.0.0

This issue should appear in the changelog. Setting target version to 4.0.0.

#7 Updated by Go MAEDA 11 months ago

#8 Updated by Go MAEDA 11 months ago

  • Duplicated by Defect #19059: Wrong number of issues for a version in the roadmap added

#9 Updated by Jean-Philippe Lang 5 months ago

  • Status changed from New to Closed
  • Assignee set to Jean-Philippe Lang

#10 Updated by Jean-Philippe Lang 2 months ago

  • Project changed from Security to Redmine
  • Category set to Roadmap

Also available in: Atom PDF