Note: There are a few cases of duplicates in user autocompletion which are being worked on.

[SECURITY] Anyone can change flags and access bug summaries due to a bad check in Flag::validate() and Flag::modify()

RESOLVED FIXED in Bugzilla 2.18

Status

()

Bugzilla
Attachments & Requests
--
critical
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Frédéric Buclin, Assigned: Frédéric Buclin)

Tracking

2.17
Bugzilla 2.18
Dependency tree / graph
Bug Flags:
approval +
blocking2.20 +
approval2.18 +
blocking2.18.2 +

Details

(Whiteboard: [does not affect 2.16.x] [ready for 2.18.2] [ready for 2.20])

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

12 years ago
Flag::validate() and Flag::modify() do not check that the flag ID one tries to
change is consistent with the bug ID and/or attachment ID passed to the
function. This way, we are able to access the summary of (almost) any bug which
has flags set on it.

It's very difficult to reproduce if you have no idea of flag IDs set on a given
bug. The idea is the following:

1) hack the URL in order to have something like:

process_bug.cgi?id=196&product=Bugzilla&component=...&flag-481=%3F&requestee-481=guest%40foo.com

Here, the flag with ID = 481 is not related to the public bug 196 but is related
to another (confidential) bug. But as the URL contains the bug ID of a public
bug, neither process_bug.cgi nor Flag::validate() or Flag::modify() will
complain as they all check if the user has access to the given bug or attachment
based on the bug/attachement ID passed in the URL.


2) use the same URL as above, but change flag-481=%3F to flag-481=X.

This way, you cancel the request and the requester (in our case: a bad guy) will
receive an email that this request has been cancelled. But the email contains
the summary of the bug!! <-- Oops!!

Note that you have to use an existing flag which is either set to + or -, else
this trick won't work (if the flag is not specifically requestable!).

Note also that this does not work if you try to create a new flag using
flag_type in the URL instead of flag, because the new flag would be created in
the public bug (the one given in the URL). About this last comment, see also bug
266159.
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
Flags: blocking2.20?
Flags: blocking2.18.2?
Target Milestone: --- → Bugzilla 2.18
(Assignee)

Updated

12 years ago
Blocks: 266159
Flags: blocking2.20?
Flags: blocking2.20+
Flags: blocking2.18.2?
Flags: blocking2.18.2+
Whiteboard: [does not affect 2.16.x] [wanted for 2.18.2] [wanted for 2.20]
(Assignee)

Updated

12 years ago
Blocks: 294021
(Assignee)

Comment 1

12 years ago
Created attachment 184629 [details] [diff] [review]
patch, v1
Attachment #184629 - Flags: review?(myk)
(Assignee)

Comment 2

12 years ago
Comment on attachment 184629 [details] [diff] [review]
patch, v1

>+  [% ELSIF error == "invalid_flag_association" %]
>+    [% title = "Invalid Flag Association" %]
>+    Some flags do not belong to
>+    [% IF attach_id %]
>+      attachment [% attach_id FILTER html %].
>+    [% ELSE %]
>+      [% terms.bug %] [%+ bug_id FILTER html %].
>+    [% END %]

Note to self: it should be [%+ terms.bug %].
(Assignee)

Comment 3

12 years ago
Created attachment 184644 [details] [diff] [review]
patch, v2

Now catch inactive flag types + fix some minor nits (mainly typos).
Attachment #184629 - Attachment is obsolete: true
Attachment #184644 - Flags: review?(myk)
(Assignee)

Updated

12 years ago
Attachment #184629 - Flags: review?(myk)
Comment on attachment 184644 [details] [diff] [review]
patch, v2

Looks good. r=myk
Attachment #184644 - Flags: review?(myk) → review+
(Assignee)

Comment 5

12 years ago
Already requesting approval for the trunk. Give me a few hours to backport it to
2.18.1.
Flags: approval?
(Assignee)

Comment 6

12 years ago
Created attachment 185962 [details] [diff] [review]
patch, v2.1

FlagType.pm uses requestee_type, not requestee (stupid copy paste). Carrying
forward myk's r+ (use interdiff between v2 and v2.1 if you don't trust me).
Attachment #184644 - Attachment is obsolete: true
Attachment #185962 - Flags: review+
(Assignee)

Comment 7

12 years ago
Created attachment 185974 [details] [diff] [review]
patch for 2.18.1, v1

backport for the 2.18 branch. The code is a bit easier as 2.18 does not let you
add flags when creating a new attachment.
(Assignee)

Updated

12 years ago
Attachment #185974 - Flags: review?(myk)

Updated

12 years ago
Blocks: 297749
Comment on attachment 185974 [details] [diff] [review]
patch for 2.18.1, v1

Looks good, r=myk
Attachment #185974 - Flags: review?(myk) → review+
I missed this in my initial review, and it's nit, but if it's not too much
trouble could you change "Edition of Flags" to "Flag Editing" upon check-in and
then attach the patches with that update?  The current syntax is technically
valid but uncommon and considered clunky, and making this change in new patches
on this bug would save having to deal with that minor issue in a separate bug.
(Assignee)

Comment 10

12 years ago
ok, I will fix this nit on checkin. Patches ready for approval! :)
Flags: approval2.18?
Whiteboard: [does not affect 2.16.x] [wanted for 2.18.2] [wanted for 2.20] → [does not affect 2.16.x] [ready for 2.18.2] [ready for 2.20]

Updated

12 years ago
Severity: normal → critical
Version: 2.19.2 → 2.17
(Assignee)

Comment 11

12 years ago
For further information about this bug, see my comment in bug 297749 comment 4.
(Assignee)

Comment 12

12 years ago
Created attachment 188625 [details] [diff] [review]
patch for trunk, v2.1.1

myk's nit fixed + unbitrot
Attachment #185962 - Attachment is obsolete: true
Attachment #188625 - Flags: review+
(Assignee)

Comment 13

12 years ago
Created attachment 188626 [details] [diff] [review]
patch for 2.18.1, v1.0.1

myk's nit fixed.
Attachment #185974 - Attachment is obsolete: true
Attachment #188626 - Flags: review+
Flags: approval?
Flags: approval2.18?
Flags: approval2.18+
Flags: approval+

Comment 14

12 years ago
Tip:

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.89; previous revision: 1.88
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.263; previous revision: 1.262
done
Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.45; previous revision: 1.44
done
Checking in Bugzilla/FlagType.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v  <--  FlagType.pm
new revision: 1.19; previous revision: 1.18
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.52; previous revision: 1.51
done

2.18:

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.58.2.6; previous revision: 1.58.2.5
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.205.2.22; previous revision: 1.205.2.21
done
Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.18.2.7; previous revision: 1.18.2.6
done
Checking in Bugzilla/FlagType.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v  <--  FlagType.pm
new revision: 1.7.2.2; previous revision: 1.7.2.1
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.39.2.6; previous revision: 1.39.2.5
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Summary: Anyone can access bug summaries due to a bad check in Flag::validate() and Flag::modify() → [SECURITY] Anyone can change flags and access bug summaries due to a bad check in Flag::validate() and Flag::modify()

Comment 15

12 years ago
Removing from webtools-security because the advisory has been posted and the
release is complete.
Group: webtools-security
You need to log in before you can comment on or make changes to this bug.