472362 - [SECURITY] Malicious attachments can change your user settings (user + email prefs, shared searches)

Status: RESOLVED FIXED

(Bugzilla :: User Accounts, defect)

Description

[Frédéric Buclin]

If an HTML attachment contains an iframe pointing to userprefs.cgi, it can edit all your user + email prefs as well as your shared searches. userprefs.cgi should be protected by session tokens to prevent this kind of attack. Fortunately, these malicious attachments cannot change your password or your email address as your password is required.

Comment 1

[Dave Miller [:justdave]]

Isn't this a duplicate or dependency of bug 26257?

Comment 2

[Frédéric Buclin]

Not a dupe, no. Bug 26257 is about process_bug.cgi; this one is about userprefs.cgi. And we will probably use session tokens here, which is different from on-the-fly tokens used in bug 26257, so this bug doesn't depend on the other one.

Comment 3

[Frédéric Buclin]

Attachment: patch, v1

Comment 4

[Teemu Mannermaa (:wicked)]

Comment on attachment 355877 [details] [diff] [review]
patch, v1

Simple but effective against few test cases I could think of. I'm sure this gives same level of protection to userprefs as our other session token protected actions already have.

Patch also doesn't prevent changing prefs, not even multiple times in a row. Overriding works except for password and email changes since old password gets lost. I don't think that matters since you can always just reload the enter form (and changing email before token expires isn't allowed either).

Since this is first non-edit*.cgi script that uses check_token_data and related admin/confirm-action.html.tmpl template the term "administrative form" in line 32 of that template might not be entirely accurate now. I'm not going to hold review for that, though.

Comment 5

[Frédéric Buclin]

Let's take it for 3.3.2 & co as it's ready.

Comment 6

[Frédéric Buclin]

tip:

Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.126; previous revision: 1.125
done
Checking in template/en/default/account/prefs/prefs.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/prefs.html.tmpl,v  <--  prefs.html.tmpl
new revision: 1.31; previous revision: 1.30
done


3.2:

Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.120.2.2; previous revision: 1.120.2.1
done
Checking in template/en/default/account/prefs/prefs.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/prefs.html.tmpl,v  <--  prefs.html.tmpl
new revision: 1.30.2.1; previous revision: 1.30
done


3.0.6:

Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.112.2.5; previous revision: 1.112.2.4
done
Checking in template/en/default/account/prefs/prefs.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/prefs.html.tmpl,v  <--  prefs.html.tmpl
new revision: 1.27.2.1; previous revision: 1.27
done


2.22.6:

Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.95.2.1; previous revision: 1.95
done
Checking in template/en/default/account/prefs/prefs.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/prefs.html.tmpl,v  <--  prefs.html.tmpl
new revision: 1.21.2.2; previous revision: 1.21.2.1
done

Comment 7

[Max Kanat-Alexander]

Removing this bug from the security group, as the Security Advisory was sent (bug 468249)

Comment 9

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/qa/4.2/
modified t/test_security.t
Committed revision 208.

Committing to: bzr+ssh://[e-mail]/bugzilla/qa/4.0/
modified t/test_security.t
Committed revision 197.

Committing to: bzr+ssh://[e-mail]/bugzilla/qa/3.6/
modified t/test_security.t
Committed revision 155.