1004843 - [Security Review][Fuzzing][LangFuzz] Replace YARR with irregexp

Status: RESOLVED FIXED

(mozilla.org :: Security Assurance: Review Request, task)

Description

[Christian Holler (:decoder)]

JS fuzzing was requested for bug 976446. Goal is to find and fix potential regressions before landing.

Comment 1

[Christian Holler (:decoder)]

Testing this on langfuzz1 now with 32/64 bit debug+opt and opt builds.

Brian, I assume this will also need ARM specific testing right? Should I run this patch also through the ARM simulator?

Comment 2

[Brian Hackett [Laid off!]]

Eventually, yeah, but right now ARM (including the simulator) seems to be broken; I'll fix this by early next week.

Comment 3

[Christian Holler (:decoder)]

Found four issues in the first round of fuzzing, second round requested. Second round should also include ASan fuzzing to find potential problems within irregexp itself.

Brian, is the ARM issue fixed now so we can test this on ARM as well?

Comment 4

[Brian Hackett [Laid off!]]

(In reply to Christian Holler (:decoder) from comment #3)
> Found four issues in the first round of fuzzing, second round requested.
> Second round should also include ASan fuzzing to find potential problems
> within irregexp itself.
> 
> Brian, is the ARM issue fixed now so we can test this on ARM as well?

I've run this in the ARM simulator and I get a couple MOZ_CRASH()'s on jit-tests in Simulator-arm.cpp code (at lines 1939 and 3999 fwiw).  I don't know if this is due to bugs in the patch or to limitations in the simulator.  Jan, do you know what could be the problem here?  Anyways, I think this is fine to go ahead and test on ARM.

Comment 5

[Jan de Mooij [:jandem]]

(In reply to Brian Hackett (:bhackett) from comment #4)
> I've run this in the ARM simulator and I get a couple MOZ_CRASH()'s on
> jit-tests in Simulator-arm.cpp code (at lines 1939 and 3999 fwiw).  I don't
> know if this is due to bugs in the patch or to limitations in the simulator.
> Jan, do you know what could be the problem here?  Anyways, I think this is
> fine to go ahead and test on ARM.

Did your ARM fixes (bug 976446 comment 31) address this too?

Comment 6

[Brian Hackett [Laid off!]]

(In reply to Jan de Mooij [:jandem] from comment #5)
> (In reply to Brian Hackett (:bhackett) from comment #4)
> Did your ARM fixes (bug 976446 comment 31) address this too?

Yes, this was due to the ABI bug when growing the backtrack stack that caused us to end up executing random memory and make the simulator angry.

Comment 7

[Christian Holler (:decoder)]

This landed :)

Comment 8

[Brian Hackett [Laid off!]]

This hasn't landed yet, so far just a small build break fixing patch has gone in.