927736 - "invalid token" error if someone else changes the CC list while viewing a bug

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[:glob ✱]

steps:

1. load the same bug in two browsers
2. on browser A, adjust the CC list and submit
3. on browser B, adjust another field and submit

expected:

submission on browser B works without error or mid-air warning.

actual:

"illegal token" error.



what's happening is the token provided in the post data is validated against the bug's current delta_ts, which was updated by browser A.  this wasn't a problem as all changes triggered a mid-air stop, however bug 69447 changed that.

Comment 1

[:glob ✱]

Attachment: 927736_1.patch

this makes us validate the token against the delta_ts at the time it was issued, rather than the current value.

note - delta_ts is provided by the POST data (as is the token we're validating), however as far as i can tell this is safe as we're hashing with the site_wide_secret.

Comment 2

[Frédéric Buclin]

In fact, nobody detected this bug before because the security bug hide this problem as process_bug.cgi was always recreating the token if a potential midair collision was possible. That's why 4.4 was working "fine".

Comment 3

[Frédéric Buclin]

Comment on attachment 818255 [details] [diff] [review]
927736_1.patch

If I intentionally omit delta_ts from the list of parameters to skip the midair collision page, the token check now triggers:

  process_bug.cgi: Use of uninitialized value $args[3] in join or string at Bugzilla/Token.pm line 170

That's because you are concatenating an undefined string. To fix the problem, make sure $delta_ts is always defined, see line 114:

  my $delta_ts = $cgi->param('delta_ts') || '';


r=LpSolit with this fix on checkin.

Comment 4

[Frédéric Buclin]

Approvers are all away and this patch must be checked in asap so that we can re-tag bzr branches. Abusing my privileges... a=LpSolit

Comment 5

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified process_bug.cgi
Committed revision 8783.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified process_bug.cgi
Committed revision 8629.