268158 - XSLT translation code doesn't handle infinite loop, requires you to kill the browser

Status: RESOLVED WORKSFORME

(Core :: XSLT, defect)

Description

[Ken Wronkiewicz]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20041001 Firefox/0.10.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; rv:1.7.3) Gecko/20041001 Firefox/0.10.1

I'm goofing around with XSLT in Mozilla Firefox.  I discovered that the browser
doesn't, at all, handle infinite loops (except on the PowerPC where they execute
in 2.2 seconds ;) ) gracefully.

So, what happened is that I accidentally created a XSLT stylesheet with an
infinite loop.  When I try to load the resulting page, the browser locks up
hard.  The only recourse is to kill the process.

Reproducible: Always
Steps to Reproduce:
Load navbar.opml from the attached ZIP file.
Actual Results:  
Browser locks up.

Expected Results:  
Given that saying "No XSLT sheet should recurse more than x times" is bad, the
XSLT code should not be able to lock up the machine and the STOP button should
still function.

Attached is a ZIP of the test case.

Also, just because it could be considered a DoS attack, I'm marking it as security.

Comment 1

[Ken Wronkiewicz]

Attachment: The files necessary to reproduce the bug

navbar.opml is the file that crashes Mozilla when it's loaded
navbar.xsl is the XSLT style sheet -- this is where the bad code lies
opml.dtd is the DTD for OPML.

Comment 2

[Jesse Ruderman]

Is Mozilla's XSLT code ever used by servers?  If not, this bug should be public,
because it's just a client DoS.

Comment 3

[timeless]

there were reports of transformiix being used by servers....

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This WORKSFORME

We already have code that says 'recursion more then 20000 times is bad'. Running
the attachment on a 1.7 build doesn't crash but rather show me an errormessage
about recursion going out of hand.

Maybe you didn't wait long enough?

Comment 5

[Daniel Veditz [:dveditz] (OOO: back Aug 26)]

WFM, too. Took about 30 seconds on a 2Ghz laptop, but eventually I get the
recursion error message when it gives up.

Comment 6

[Peter Van der Beken [:peterv]]

30 seconds seems like a lot. Maybe we should lower the limit or add a time-based
limit like our DOM code has?

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

A timebased limit seems like a good idea. This way we would take care of the
case where each iteration adds a lot of elements. Our DOM doesn't seem to scale
too well (for good reason) which is probably causing the long delay. Would be
interesting to see a profile of where we're spending time...

Having code like
if (mRecursionDepth > 1000 && mRecursionDepth % 100 == 0 &&
    timeLimitExceeded()) {
    return NS_ERROR_XSLT_BAD_RECURSION;
}

should ensure that we don't loose too much speed or fail on slow but reasonable
stylesheets.