Closed Bug 854897 Opened 7 years ago Closed 6 years ago

Medium exploitable crash [@ IndicShape] with EXCEPTION_STACK_BUFFER_OVERRUN [WinXP]

Categories

(Core :: Graphics, defect, critical)

18 Branch
x86
Windows XP
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla26
Tracking Status
firefox21 - wontfix
firefox22 - wontfix
firefox23 - wontfix
firefox24 + fixed
firefox25 + fixed
firefox26 --- fixed
firefox-esr17 24+ wontfix
b2g18 --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- unaffected
b2g-v1.4 --- unaffected

People

(Reporter: bc, Assigned: jfkthame)

References

(Blocks 1 open bug, )

Details

(5 keywords, Whiteboard: [adv-main24+][MSRC case 14349st -- keep hidden until MS is clear too])

Crash Data

Attachments

(4 files, 1 obsolete file)

1. http://www.quraneralo.com/our-duties-towards-sahabah/ on Windows XP Firefox 18, Beta/20, Aurora/21, Nightly/22. 

Crash opt:
[@ IndicShape(HDC__*, void**, unsigned short const*, int, int, tag_SCRIPT_ANALYSIS*, unsigned short*, unsigned short*, tag_SCRIPT_VISATTR*, int*) ] 

0 	usp10.dll 	IndicShape 	
1 	xul.dll 	gfxUniscribeShaper::ShapeText 	gfx/thebes/gfxUniscribeShaper.cpp:490
2 	xul.dll 	gfxShapedText::SetupClusterBoundaries 	gfx/thebes/gfxFont.cpp:4557
3 	mozalloc.dll 	moz_xrealloc 	memory/mozalloc/mozalloc.cpp:86
4 	xul.dll 	gfxFontGroup::InitScriptRun<wchar_t> 	gfx/thebes/gfxFont.cpp:3951
5 	xul.dll 	gfxScriptItemizer::Next 	gfx/thebes/gfxScriptItemizer.cpp:228

exploitability: medium
EXCEPTION_STACK_BUFFER_OVERRUN

bp-2008e109-b8ca-43d3-9acd-32b332130326
bp-905e95fe-e369-4cdf-89f7-4bc302130326

Crash debug:
usp10.dll@0x31a31 gfxUniscribeShaper::ShapeText nsAutoTArray<gfxShapedText::DetailedGlyph, int>::~nsAutoTArray<gfxShapedText::DetailedGlyph, int> gfxHarfBuzzShaper::SetGlyphsFromRun

exploitability: medium
EXCEPTION_STACK_BUFFER_OVERRUN
Attached file testcase
Keywords: testcase
I'm guessing there's a single word in that testcase that is sufficient to trigger the crash - any chance you could isolate it?
sure. brb.
Attached file testcase2 (obsolete) —
Attached file testcase2
without cruft.
Attachment #729878 - Attachment is obsolete: true
It may not be much comfort, but FWIW the testcase also crashes Internet Explorer just as readily - see screenshot.

This looks like a Uniscribe bug, and I don't think we can realistically do much to work around it except switch Indic shaping on Windows (at least on XP - does Vista or Win7's version of Uniscribe also crash?) over to harfbuzz. Note that if you set gfx.font_rendering.harfbuzz.scripts to -1, it will no longer crash.

I've been reluctant to make that switch until we have more extensive and automated regression testing in place for the harfbuzz Indic shaper, as there's the possibility that it could regress shaping behavior for some fonts. But this gives us some extra incentive to move forward there.
(In reply to Jonathan Kew (:jfkthame) from comment #6)

> does Vista or Win7's version of Uniscribe also crash?

In crash automation I test 32bit builds on winxp and win7 32bit and 64bit. Only xp crashed. I do have Vista locally as a virtualbox vm. I'll try to test that later today.
To test this on vista or win7, you'll need to ensure that hardware acceleration is -not- enabled; otherwise Indic shaping will go via directwrite rather than uniscribe, so you may not run into the issue. Does win7 crash with hw/acc disabled?
My vms are vanilla VMware ESXi. I don't believe they have any hardware acceleration available. I have some virtualbox vms that I might be able to toggle acceleration. I'll get to that later today.
I did not crash with either Vista or Win7.

dveditz: re comment 6: can we contact Microsoft about this?
I don't know why a plain stack buffer overflow wouldn't be fully exploitable.

Yeah, I'll contact MS.
Summary: Medium exploitable crash [@ IndicShape] with EXCEPTION_STACK_BUFFER_OVERRUN → Medium exploitable crash [@ IndicShape] with EXCEPTION_STACK_BUFFER_OVERRUN [WinXP]
Microsoft is tracking this as "case 14349st" (the "st" apparently refers to the case manager Stephen). Please don't unhide this bug even after our fix until we check with them.
Whiteboard: [MSRC case 14349st -- keep hidden until MS is clear too]
Just reproduced on Windows XP using the public url

bp-15531d08-5cd0-4412-ac32-af92d2130418
bp-cb996650-d5d5-4467-8627-921bf2130418

stefin, is there anything else you need?
Flags: needinfo?(stefin)
(In reply to Daniel Veditz [:dveditz] from comment #12)
> Microsoft is tracking this as "case 14349st" (the "st" apparently refers to
> the case manager Stephen). Please don't unhide this bug even after our fix
> until we check with them.

Do we want to track for our releases, even though it sounds like this won't be fixed in Firefox? Or are we waiting on MS investigation prior to assigning an engineer here?
This won't directly be "fixed" in Firefox, but it will be avoided when we switch the text back-end on Windows to harfbuzz for all scripts, thus bypassing Uniscribe shaping altogether (bug 797405). That in turn is dependent on having sufficiently extensive testing in place (bug 770591).
Depends on: 797405
Sounds like we don't need to track for upcoming releases for the time being, since this will require major change in bug 797405.
Hi Jonathan do we have the harfbuzz bug roadmapped?
Flags: needinfo?(jfkthame)
Well - "roadmapped" is perhaps a bit too formal for the current state of things.

The actual change we need to make for Gecko in bug 797405 is trivial - it's just switching a pref. But before we do that, we'd like to have a *really* extensive test suite in place for complex-script shaping behavior.

Behdad and I have discussed this from time to time, gradually refining some ideas and tools to support the testing process, but we're not yet at the point of having an automated, adequately scalable setup.

(Bug 770591 was filed to track the testing issue, though I think it's possible we'll end up with the main shaping regression-test suite being run outside Mozilla's infrastructure, e.g. at Google instead.)
Flags: needinfo?(jfkthame)
If the only fix we're pursuing to resolve this bug is bug 797405, then we won't track this bug for upcoming releases.
stefin, 

any update on this from your end?
can you reproduce?
do you agree this is exploitable?
do you intend to fix this?
Renominating this one because we need to fix it. Tracking a pref-flip bug seems unmotivated and I don't want to explain why we need it in a public bug. I also don't see any movement on the tests mentioned in comment 18 so we should just flip it and get real-world testing ASAP.
Given that this issue is only on WinXP, not later Windows releases (comment 10), I'd be reluctant to push a risky change to *all* Windows users in order to address it. Maybe we should look into switching XP users (only) from Uniscribe to HarfBuzz.

This would require a code-level patch, but it should be a trivial one - we already have code in the platform that checks the Windows version at runtime, so we'd just need to connect that to the code in gfxGDIFont::ShapeText that chooses which shaper backend to call.
This should work to switch XP users to harfbuzz shaping (for Indic scripts), without affecting Win8/8 users. There's still a risk it could regress shaping behavior for specific fonts and/or text strings; harfbuzz has generally had more testing with newer (Win7) fonts than with the older XP versions, or with the mass of third-party fonts (of varying vintages) out there, which is why I'm hesitant to simply toggle the pan-Windows default in bug 797405 at this point. Tryserver run in progress at https://tbpl.mozilla.org/?tree=Try&rev=8b1571105aa0.
Attachment #766471 - Flags: review?(jdaggett)
Assignee: nobody → jfkthame
Oops. "...without affecting Win8/8 users" should of course refer to Win7/8.
I think before landing this we should do proper triage on versions of the Uniscribe DLL to confirm that the OS version is the right way to classify this.  Microsoft uses multiple update vectors to make changes to Uniscribe (e.g. Office, VOLT, Language Packs, Security Fixes, OS updates).  I think it's possible that this bug may occur with a given flavor of Windows that is *not* strictly XP (e.g. early versions of Vista).

If the version of the Uniscribe DLL containing the problem *only* exists on XP than the logic in the patch makes sense.  Otherwise we may need to use the Uniscribe DLL version instead.

For more information see:
http://en.wikipedia.org/wiki/Uniscribe
As far as I'm aware, it seems unlikely that significant numbers of users will be running *older* versions of Uniscribe than the default installed with their Windows platform, and thus remain vulnerable even though their OS version suggests they shouldn't be. Given that Bob reported that Vista did not crash, I think making the switch for WinXP only is a simple way to address the problem for the vast majority of affected users. It's admittedly a "broad brush" that will include users who have actually upgraded their Uniscribe and so don't really need the fix, but that's a minor issue.

It's true that the USP10.dll version would be a more accurate indicator of whether a given system may be vulnerable, but I'm not in a position to attempt that investigation. Do you have a collection of all the Uniscribe versions as distributed through these various channels?

Given that the simple approach here will resolve the issue for (at least!) the vast majority of affected users, I don't see much justification for spending more time on this.
I think it should be a pretty simple process for a QA person with access to a MSDN subscription.  Just download a smattering of the affected OS builds (e.g. WinXP SPx, Vista RC, Win7 RC), check the Uniscribe DLL version and see if the testcase crashes.  With that info in hand we can then make the call as to whether a simple OS version check is sufficient.

Jet, maybe you can suggest someone to do this?
Flags: needinfo?(bugs)
I'm inclined to accelerate the migration to HarfBuzz for shaping these scripts.
Flags: needinfo?(bugs)
(In reply to Jet Villegas (:jet) from comment #28)
> I'm inclined to accelerate the migration to HarfBuzz for shaping these
> scripts.

Jet, the question is whether the problem here is limited to XP or not.  The patch uses the OS version number (i.e. XP or not) to decide whether to force on Harfbuzz shaping.  But as you can see from the Wikipedia page for Uniscribe, the Uniscribe DLL is updated a lot.  The question is whether there's a version of Uniscribe that shipped with early versions of OS's *other* than XP that would exhibit the problem.

What I'm asking for is a QA resource to do a more complete analysis of which versions of Uniscribe are affected by this bug to determine whether those versions are strictly limited to XP or not.  If they are, then the patch is fine, otherwise we need to base the switch on the Uniscribe DLL version instead.
Alternatively, we could

(1) take the existing patch for now, which addresses the issue for the large class of users that we -know- are affected;

(2) not worry much about the possibility that there -might- be other marginal affected populations (such as users on early Vista releases who have avoided updating their OS);  if such cases do in fact exist, we (a) won't be making things any worse for them, and (b) can't really be held responsible for the fact that they've chosen to stay on a vulnerable OS version.

(3) focus efforts on the testing that will enable us to make the global switch for -all- users with confidence that we're not regressing behavior.
RE: QA resource. I've assigned myself as QA owner, but I may be recruiting Kamil Jozwiak for this task as well.
QA Contact: mwobensmith
(In reply to Jonathan Kew (:jfkthame) from comment #30)
> Alternatively, we could
> 
> (1) take the existing patch for now, which addresses the issue for the large
> class of users that we -know- are affected;
> 
> (2) not worry much about the possibility that there -might- be other
> marginal affected populations (such as users on early Vista releases who
> have avoided updating their OS);  if such cases do in fact exist, we (a)
> won't be making things any worse for them, and (b) can't really be held
> responsible for the fact that they've chosen to stay on a vulnerable OS
> version.
> 
> (3) focus efforts on the testing that will enable us to make the global
> switch for -all- users with confidence that we're not regressing behavior.

:jfkthame, can you please needinfo the right folks who can help on your comments ?

In addition, is this targeted to be resolved in FX24 timeframe which is the main reason I tracked this.If we are unsure of its resolution, I'll go ahead and untrack this at this point as this security bug has been there long enough and we don't have  track for a particular release unless a blocker (needinfo : dan).
Flags: needinfo?(dveditz)
(In reply to Jonathan Kew (:jfkthame) from comment #30)
> Alternatively, we could
> 
> (1) take the existing patch for now, which addresses the issue for the large
> class of users that we -know- are affected;
> 
> (2) not worry much about the possibility that there -might- be other
> marginal affected populations (such as users on early Vista releases who
> have avoided updating their OS);  if such cases do in fact exist, we (a)
> won't be making things any worse for them, and (b) can't really be held
> responsible for the fact that they've chosen to stay on a vulnerable OS
> version.
> 
> (3) focus efforts on the testing that will enable us to make the global
> switch for -all- users with confidence that we're not regressing behavior.

Um, there isn't a big effort to figure out the details needed, namely which versions of Uniscribe are affected and do they only ship with XP.  Better to figure this out first rather than patch one way and have to patch again once we do the required testing.
(In reply to bhavana bajaj [:bajaj] from comment #32)
> (In reply to Jonathan Kew (:jfkthame) from comment #30)
> > Alternatively, we could
> > 
> > (1) take the existing patch for now, which addresses the issue for the large
> > class of users that we -know- are affected;
> > 
> > (2) not worry much about the possibility that there -might- be other
> > marginal affected populations (such as users on early Vista releases who
> > have avoided updating their OS);  if such cases do in fact exist, we (a)
> > won't be making things any worse for them, and (b) can't really be held
> > responsible for the fact that they've chosen to stay on a vulnerable OS
> > version.
> > 
> > (3) focus efforts on the testing that will enable us to make the global
> > switch for -all- users with confidence that we're not regressing behavior.
> 
> :jfkthame, can you please needinfo the right folks who can help on your
> comments ?

I'm not sure what further input to seek, aside from the testing of uniscribe versions that jdaggett wants to see done.

I am currently working on point (3) above, with the aim of switching globally from uniscribe to harfbuzz shaping once we have tested that more extensively. That's still likely to be some weeks off, however, given the scale of testing involved in covering many scripts, fonts and texts. Maybe for Fx26, if things go well.

> In addition, is this targeted to be resolved in FX24 timeframe which is the
> main reason I tracked this.

We could take the current patch for Fx24/25; that would resolve the crash for XP users - who are the only ones we know to be affected. If QA can complete more detailed testing across Windows/uniscribe versions, maybe we could refine it further, but I don't see why that should block us from taking the fix based on what we already know.
Considering current events and the lack of responses in this bug, I believe it is highly likely that this is being exploited in the wild and will remain unfixed. In this case the pursuit of the perfect is the enemy of the good. If we have a fix in hand that even partially protects our users without disclosing the issue, we should take it now.
Matt, QA needs to do something here per comment 31.
Flags: needinfo?(dveditz)
I'll look into this and post all my results ASAP
Hows the investigation going?
I was away for work last week and didn't get a chance to take a look at this. I will have a look at this and post my results sometime this week.
I've been trying to reproduce the issue on Windows XP Pro x64 before going through all the different OS's but couldn't reproduce the problem. I turned off hardware acceleration but still couldn't get the crash. I would like to reproduce this so I know I am doing it correctly before going through all the other OS's.

Went through the following:

- loaded Windows XP Pro SP2 x64 (no other updates) in VirtualBox (VM instance)
- turned off hardware acceleration (also tried with hardware acceleration turned on)
- Installed Firefox 23/24/25 and then loaded "testcase2" without any crashes (just loads a square symbol)
- Attempted to install Firefox on Windows XP with something earlier then SP2 but received an error message indicating SP2 is needed

Please let me know if I am doing something wrong or if someone can point me in the right direction it would be much appreciated!
(In reply to Kamil Jozwiak [:kjozwiak] from comment #40)
> I've been trying to reproduce the issue on Windows XP Pro x64 before going
> through all the different OS's but couldn't reproduce the problem. I turned
> off hardware acceleration but still couldn't get the crash. I would like to
> reproduce this so I know I am doing it correctly before going through all
> the other OS's.
> 
> Went through the following:
> 
> - loaded Windows XP Pro SP2 x64 (no other updates) in VirtualBox (VM
> instance)

The bug was reported for x86. I think it's likely it will behave the same on x64 (as Firefox will still be an x86 application, linking to 32-bit libs), but haven't actually tried that.

> - turned off hardware acceleration (also tried with hardware acceleration
> turned on)
> - Installed Firefox 23/24/25 and then loaded "testcase2" without any crashes
> (just loads a square symbol)

This is the main problem, I think - it sounds like your WinXP doesn't have the relevant fonts and Indic script support installed. IIRC, the support for these languages (in particular, Bengali) is an optional component on XP, not part of the default en-US install. So try:

- Go to Control Panel, then Regional and Language Options
- In the Language tab, select Install files for complex script and right-to-left languages (including Thai)

With the necessary support installed, "testcase2" should display a Bengali letter, not just a missing-glyph box (unless it crashes, of course!).
On the XP installs I have done, as far as I know they were vanilla installations and I didn't intentionally choose additional fonts. The XP installs were all 32bit and the version of Firefox was 32bit as well.
Thanks Jonathan! Enabling/Installing those complex scripts through the Regional & Language Options reproduced the crash on Win XP SP2 x86:

- Windows XP Pro x86 SP2 (fresh install without any updates) - Reproduced the crash with the latest Firefox 25 (Nighty Channel)

- Windows XP Pro x64 SP2 (fresh install without any updates) - Couldn't reproduce the crash, still loaded the little square even after installing the "Complex Scripts" through the Regional & Language Options. Also tried installing the Asian languages just in case but still couldn't reproduce.

Does this mean the issue isn't present in Win XP SP2 x64? (just worried as the font is not appearing and still showing a square but Win XP SP2 x86 crashed as soon as testcase2 was opened within the browser)
(In reply to Kamil Jozwiak [:kjozwiak] from comment #43)
> Thanks Jonathan! Enabling/Installing those complex scripts through the
> Regional & Language Options reproduced the crash on Win XP SP2 x86:
> 
> - Windows XP Pro x86 SP2 (fresh install without any updates) - Reproduced
> the crash with the latest Firefox 25 (Nighty Channel)
> 
> - Windows XP Pro x64 SP2 (fresh install without any updates) - Couldn't
> reproduce the crash, still loaded the little square even after installing
> the "Complex Scripts" through the Regional & Language Options. Also tried
> installing the Asian languages just in case but still couldn't reproduce.
> 
> Does this mean the issue isn't present in Win XP SP2 x64? (just worried as
> the font is not appearing and still showing a square but Win XP SP2 x86
> crashed as soon as testcase2 was opened within the browser)

Seeing the "little square" suggests that the x64 version, even after you enabled "complex scripts", still does not include the Bengali font (I think it's Vrinda) that is shipped with the x86 version. Could you check the Fonts folder and see if it's present?

If you copy vrinda.ttf from the x86 installation to the x64 version, does it -then- crash? (It's possible it won't, because if they're not shipping the font, perhaps this means uniscribe on that system doesn't even attempt to support Bengali. In which case it'd be unlikely to hit the crash.) Or does it display a Bengali character successfully?
Jonathan,

- Checked the C:\Windows\Fonts\ directory under Windows XP SP2 x64 and didn't see vrinda.tff
- Moved vrinda.tff from the Windows XP SP2 x86 machine into the Windows XP SP2 x64 machine
- Ran through "testcase2" without any issues, the character appeared without any problems (tried several times)
- Tried it with Hardware Acceleration On & Off, browser under x64 never crashed (with vrinda.tff inside the fonts folder)
- Looks like its crashing in x86 but not under x64

Would this be valid for testing on the other OS's? Checking to see if that font exists and if it doesn't, add it from the Windows XP x86 installation and run through "testcase2"?

Appreciate the help!
(In reply to Kamil Jozwiak [:kjozwiak] from comment #45)
> Jonathan,
> 
> - Checked the C:\Windows\Fonts\ directory under Windows XP SP2 x64 and
> didn't see vrinda.tff
> - Moved vrinda.tff from the Windows XP SP2 x86 machine into the Windows XP
> SP2 x64 machine
> - Ran through "testcase2" without any issues, the character appeared without
> any problems (tried several times)
> - Tried it with Hardware Acceleration On & Off, browser under x64 never
> crashed (with vrinda.tff inside the fonts folder)
> - Looks like its crashing in x86 but not under x64

OK - I suspect this may mean that uniscribe on the x64 version of WinXPSP2 lacks Bengali support. (You could try loading bn.wikipedia.org/wiki/ and compare the display with the x86 version - if, as I suspect, uniscribe lacks support for the script, there should be some fairly obvious differences even though the basic glyphs will be present.)

Presumably Bengali support will appear in some later x64 release - certainly it's there in Win7, but I don't know if it ever shipped for XP/x64. If it did, then the issue might arise there.

> Would this be valid for testing on the other OS's? Checking to see if that
> font exists and if it doesn't, add it from the Windows XP x86 installation
> and run through "testcase2"?

It's essential that you enable "support for complex scripts" in Windows (not just install the font), as without this, the relevant uniscribe version may not be present.

It seems likely that if "complex scripts" support for any given version does -not- give you the Bengali font, it's because that script simply isn't supported, and so the bug won't be relevant. I think that's probably the case for your x64 installation. But presumably any x86 version going forward will include that support (once "complex scripts" have been enabled) - it won't have been -removed- in a later SP or OS release.

So I guess the interesting questions are (a) at what point after XP/SP2 do Bengali-enabled ("complex scripts") x86 releases -stop- suffering from the crash; and (b) at what point does Bengali support appear on x64 with "complex scripts", and does it ever suffer from the crash?
Jonathan, went through the following test cases for Windows XP and the other OS's and only received the crash in Windows XP x86.

Everything was tested against the following build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-08-01-03-02-23-mozilla-central/

Windows XP Pro SP2 x86 (no updates): [FAILED]
- Reproduced the crash using testcase2
Windows XP Pro SP3 x86 (no updates): [FAILED]
- Reproduced the crash using testcase2
Windows XP Pro SP3 x86 (Fully Updated): [FAILED]
- Reproduced the crash using testcase2

Windows XP Pro SP2 x64 (no updates): [PASSED]
- Disabled Hardware Acceleration
- Checked the C:\Windows\Fonts for vrinda.tff (didn't exist)
- Enabled "support for complex scripts"
- Installed the vrinda.tff font by placing it into fonts
- Couldn't reproduce the issue using testcase2 (tried several times)
Windows XP Pro SP2 x64 (Fully Updated): [PASSED]
- Disabled Hardware Acceleration
- Checked the C:\Windows\Fonts for vrinda.tff (didn't exist)
- Enabled "support for complex scripts"
- Installed the vrinda.tff font by placing it into fonts
- Couldn't reproduce the issue using testcase2 (tried several times)

For the below OS's, I went through the following test cases and all of them passed without any issues:

- Disabled Hardware Acceleration
- Checked the C:\Windows\Fonts for vrinda.tff (it's installed)
- Couldn't reproduce the issue using testcase2 & character appeared correctly (tried several times)

Windows Vista Ultimate x86 (fresh install without updates): [PASSED]
Windows Vista Ultimate SP1 x86 (no updates): [PASSED]
Windows Vista Ultimate SP2 x86 (no updates): [PASSED]
Windows Vista Ultimate SP2 x86 (Fully Updated): [PASSED]

Windows Vista Ultimate x64 (fresh install without updates): [PASSED]
Windows Vista Ultimate x64 (Fully Updated without any SP's): [PASSED]
Windows Vista Ultimate SP1 x64 (no updates): [PASSED]
Windows Vista Ultimate SP2 x64 (no updates): [PASSED]
Windows Vista Ultimate SP2 x64 (Fully Updated): [PASSED]

Windows 7 Home Premium X86 (fresh install without updates): [PASSED]
Windows 7 Home Premium SP1 X86 (no updates): [PASSED]
Windows 7 Home Premium SP1 X86 (Fully Updated): [PASSED]

Windows 7 Home Premium X64 (fresh install without updates): [PASSED]
Windows 7 Home Premium SP1 X64 (no updates): [PASSED]
Windows 7 Home Premium SP1 X64 (Fully Updated): [PASSED]

The only one that I have left is Windows 8 x86/64 and will try to get that done tomorrow as I still need to create VM's for both machines. Jonathan, please let me know if there's any issues with the test that was done or if I missed something.
(In reply to Kamil Jozwiak [:kjozwiak] from comment #47)
> The only one that I have left is Windows 8 x86/64 and will try to get that
> done tomorrow as I still need to create VM's for both machines. Jonathan,
> please let me know if there's any issues with the test that was done or if I
> missed something.

I'm not really concerned about Windows 8; it seems clear this was a flaw in early Uniscribe Indic-script code, it's since been fixed, and we have no reason to suspect it would have regressed again recently.

John, are there other versions or combinations you want to see tested here? It still seems to me that taking the simplest patch, to just avoid Uniscribe on WinXP, is a reasonable solution for the time being, while we continue in-depth harfbuzz testing. There's no indication here of problems on post-XP systems.
Flags: needinfo?(jdaggett)
Comment on attachment 766471 [details] [diff] [review]
prefer harfbuzz shaping for Indic text on WinXP

Yeah, given the testing results in comment 47 this seems reasonable to me.
Attachment #766471 - Flags: review?(jdaggett) → review+
Flags: needinfo?(jdaggett)
Comment on attachment 766471 [details] [diff] [review]
prefer harfbuzz shaping for Indic text on WinXP

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Not readily, IMO; though if it prompted someone to try fuzz-testing on XP with Indic text and fonts, they'd probably hit the crashing case before long.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

The patch just shows that we are avoiding the use of Uniscribe for Indic text, but does not give any clue to the reason for this, or the specific font(s) or text sequence(s) that lead to the crash.

Which older supported branches are affected by this flaw?

All - it's really a Windows XP flaw. (The patch here just aims to avoid using the buggy API, not fix it.)

If not all supported branches, which bug introduced the flaw?
n/a.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

The patch would backport easily. However, on older branches (particularly ESR17, with a much older version of harfbuzz), it's likely there would be more significant behavioral regressions. Given that the underlying bug has been present "forever" (and affects all applications, not just Firefox - see comment 6), I don't see much value in backporting.

How likely is this patch to cause regressions; how much testing does it need?

The only area that could regress is text shaping for Indic scripts on XP. It's impractical for us to proactively test every possible text sequence against every font in the wild, so there almost certainly -will- be a few regressions with particular fonts and text sequences, although we believe the vast majority of text will render fine. (There will also be -improvements- in some cases, though that will be small comfort for anyone who does encounter a shaping regression.)
Attachment #766471 - Flags: sec-approval?
Comment on attachment 766471 [details] [diff] [review]
prefer harfbuzz shaping for Indic text on WinXP

sec-approval+ for trunk. 

I would go ahead and make the Aurora patch and nominate it. This is tracking+ for that so it should be approved immediately.
Attachment #766471 - Flags: sec-approval? → sec-approval+
Comment on attachment 766471 [details] [diff] [review]
prefer harfbuzz shaping for Indic text on WinXP

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a (workaround for WinXP bug)

User impact if declined: crash (possibly exploitable?) may be triggered by specific characters in a web page

Testing completed (on m-c, etc.): just landed on inbound; tested with local build to confirm this avoids the crash

Risk to taking this patch (and alternatives if risky): see comment 50 last para

String or IDL/UUID changes made by this patch: none


Note that this patch landed on inbound for mozilla-26, as review and sec-approval only happened on merge day (too late to catch the mozilla-25 train). So this nomination should therefore be considered as being for aurora-25 and beta-24, although at this time I believe those version bumps may still be pending.
Attachment #766471 - Flags: approval-mozilla-beta?
Attachment #766471 - Flags: approval-mozilla-aurora?
Attachment #766471 - Flags: approval-mozilla-beta?
Attachment #766471 - Flags: approval-mozilla-beta+
Attachment #766471 - Flags: approval-mozilla-aurora?
Attachment #766471 - Flags: approval-mozilla-aurora+
It appears this landed with today's patch tuesday:

https://technet.microsoft.com/en-us/security/bulletin/ms13-060
Certainly sounds like it; Kamil, could you re-test with a patched WinXP (and an *UN*patched Firefox version, obviously) to confirm that the testcase no longer crashes?

(It's interesting to note that the MS bulletin lists WinXP/x64 as being affected, although AFAICT we were never able to reproduce this specific crash on a 64-bit version.)
Flags: needinfo?(kamiljoz)
I retested my urls on unpatched windows xp 32bit and could not reproduce with beta, aurora or nightly. :-) I'll recheck after I do patch tuesday maintenance and update all my vms.
Jonathan, sure! I will check when I get home from work and post the results for both Windows XP x86/x64 versions with the latest patches + unpatched Firefox.

Ya, noticed that also, I couldn't reproduce it on Windows XP x64 and tried many times with different VM's but MS added it to the affected list...
i was not able to reproduce any crashes with the patched Firefox builds and patched XP systems.
Flags: needinfo?(stefin)
(In reply to Bob Clary [:bc:] from comment #60)
> i was not able to reproduce any crashes with the patched Firefox builds and
> patched XP systems.

And with UNpatched Firefox (e.g. FF23 release) on fully-patched XP?
see comment 59
I will also go through this tonight and post the results
Used the following build: (an older version as recommended per comment 61)

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013/04/2013-04-04-03-08-59-mozilla-central/

- Reproduced the original issue with Windows XP Pro x86 SP3 that wasn't updated with the newer patches ("hardware acceleration disabled" & enabled "support for complex scripts")
- Once the issue was reproduced, restored the Windows XP Pro SP3 x86 VM to its original state (without complex scripts, latest patches & Firefox uninstalled)
- Updated the VM instance with all of the latest patches from the Microsoft Update page
- Enabled "support complex scripts" & "hardware acceleration" and restarted the machine
- Downloaded and installed the above build
- Once installed, opened Testcase2 without any crashes nor issues
Flags: needinfo?(kamiljoz)
Shouldn't we take this on ESR17?
(In reply to Al Billings [:abillings] from comment #65)
> Shouldn't we take this on ESR17?

I don't think so (see comment 50 for details), especially given that MS has just patched the bug in XP anyway.
Whiteboard: [MSRC case 14349st -- keep hidden until MS is clear too] → [adv-main24+][MSRC case 14349st -- keep hidden until MS is clear too]
Group: core-security
You need to log in before you can comment on or make changes to this bug.