Last Comment Bug 850069 - Disable DirAuto for Firefox 20
: Disable DirAuto for Firefox 20
Status: RESOLVED FIXED
[adv-main20-]
: sec-other
Product: Core
Classification: Components
Component: Layout: Text (show other bugs)
: 20 Branch
: All All
: -- normal (vote)
: ---
Assigned To: Simon Montagu :smontagu
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-11 17:29 PDT by Daniel Veditz [:dveditz]
Modified: 2013-11-25 13:26 PST (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
+
fixed
unaffected
unaffected
unaffected
unaffected


Attachments
Backout patch (439.44 KB, patch)
2013-03-19 09:23 PDT, Simon Montagu :smontagu
ehsan: review+
Details | Diff | Splinter Review

Description Daniel Veditz [:dveditz] 2013-03-11 17:29:19 PDT
DirAuto is a bidi-related text feature supported starting in Firefox 20 (bug 548206). There have been a considerable number of regressions, many of which have been potentially exploitable security problems.

https://bugzilla.mozilla.org/buglist.cgi?keywords=regression%2C%20&keywords_type=allwords&f1=blocked&o1=equals&v1=548206

Not all of the above fixes have been backported to Firefox 20, and new ones are still coming in. bidi code has historically been fragile and this feature bears that out. It should bake for another cycle so we don't expose our users to risk.

I don't believe there is a pref for this feature. It would be risky to try to back-out all the code but maybe we could change the parser to ignore the attribute. Would that be safe? It would not be the same as the code that existed before the feature landing but hopefully the regressions are coming from us applying the feature and wouldn't happen in cases where we didn't. I haven't seen any regressions blamed on bug 548206 that did not use the DirAuto attribute so it might be a workable approach. (I should look more thoroughly though.)
Comment 1 Daniel Veditz [:dveditz] 2013-03-11 17:33:29 PDT
tracking-status20+ because release-drivers need to "track" this issue one way or another, not prejudging their decision that a fix is less risky than carrying on.

Although this bug is rated "sec-other" (it's not describing a specific security flaw) most of the security regressions that have been fixed have been sec-critical use-after-frees. If there are more bugs lurking I would expect them to be of that type and potential severity.
Comment 2 :Ehsan Akhgari 2013-03-11 20:57:42 PDT
Yes, I agree that we should do this.  Unfortunately, I can't think of an easy kill-switch.  :(  Maybe Simon can?
Comment 3 Simon Montagu :smontagu 2013-03-11 22:46:54 PDT
I agree we need another cycle before releasing this feature. There's no oneliner kill-switch, but it won't be particular hard just to back out all the code, most of which is isolated in DirectionalityUtils.
Comment 4 Alex Keybl [:akeybl] 2013-03-18 16:11:29 PDT
(In reply to Simon Montagu from comment #3)
> I agree we need another cycle before releasing this feature. There's no
> oneliner kill-switch, but it won't be particular hard just to back out all
> the code, most of which is isolated in DirectionalityUtils.

Can back this out before Tuesday afternoon? a=akeybl

We don't have an opinion on disabling versus backing out. Disabling is probably lower risk functionally, but backing out may be lower risk security-wise.
Comment 5 Simon Montagu :smontagu 2013-03-19 09:23:59 PDT
Created attachment 726709 [details] [diff] [review]
Backout patch
Comment 6 :Ehsan Akhgari 2013-03-19 11:13:04 PDT
Comment on attachment 726709 [details] [diff] [review]
Backout patch

Review of attachment 726709 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't do a proper review since this is a backout of all of this stuff.  If there's something that actually needs review, please let me know.
Comment 7 Simon Montagu :smontagu 2013-03-19 11:25:46 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/2192bce8badc

Note You need to log in before you can comment on or make changes to this bug.