373610 - (refdyn) [meta] Bugs found by comparing renderings with and without dynamic changes

Status: RESOLVED FIXED

(Core :: Fuzzing, defect)

Description

[Jesse Ruderman]

This reftest-based script exhaustively tests how rendering responds to simple DOM changes.  For example, it tries removing a node, forcing a complete relayout of the page, and putting the node back.    It compares the two screenshots with the node missing to ensure that the removeChild operation is handled properly, and compares the two screenshots with the node present to ensure that the insertBefore operation is handled properly.

It tests removeChild/insertBefore for every node in the document.  It also tests attribute removal/setting and text node changes.

Running it on all the reftest files (excluding the pixel-rounding directory) takes about 10 hours on my MacBook Pro, so it's too slow to be run automatically after every checkin.  About 80% of the time is spent in PNG compression.  Is there a way to avoid PNG compression in this kind of test?

It has found 15 bugs so far, mostly incorrect-rendering bugs and a few bugs in reading the style attribute.  But I'm filing this as security-sensitive for now because I'm worried it might find more severe bugs on branches and because my experience with bug 349611 makes me feel I should be cautious.

Comment 1

[Jesse Ruderman]

Attachment: refdyn.js

Comment 2

[Martijn Wargers (dead)]

This is a great idea.
It seems to me, a lot of bugs could also be found by comparing the offsetWidth/offsetHeight/computed width/computed height, not?
In that case, you could avoid the slow image comparison.

Comment 3

[Benjamin Smedberg]

Jesse, even if it takes 10 hours, we could (and probably should) still run the test nightly if we had appropriate automation. cc'ing pav/vlad about avoiding PNG compression. It probably wouldn't be that hard to add a parameter to avoid compression.

Comment 4

[Rob Campbell [:rc] (:robcee)]

cool idea, Jesse. We should figure out how to get this running regularly on the test farm. I'll talk to you later about some ideas on irc.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The best way to avoid the PNG compression would be to write a small image comparison function in C++ rather than doing the image comparison via PNG data: URLs.  It's something that's on my mental todo list for speeding up reftest, though there should probably be a bug filed on it.  (We probably would still want to convert to data: URLs when reporting failures, but that's rare.)

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Can the image comparison be done with getImageData and in JS?  Not sure what the perf would be like, but something to consider would be to use a much smaller basea size for the tests, or at least have a test flag that indicates that a test needs a large canvas vs. the default (which should be small, like 100x100).  A C++ thing could work fine as well.

Comment 7

[Ray Kiddy]

I noticed in reftest.js that setAttribute is explicitly called on the Canvas to set its height and width. Does this necessarily need to be set to the size of the window, or can it be set to capture only a small square of the window? Or, can the width of the window be set here

If this size could be made a parameter of the test, it would be most convenient.

Comment 8

[Jesse Ruderman]

Attachment: refdyn.js

Comment 9

[Jesse Ruderman]

Attachment: refdyn.zip

Now much faster thanks to bug 387132.  Also contains updated exclusions.

Comment 10

[Jesse Ruderman]

Attachment: refdyn.zip

Updated to understand the current reftest.list format.  Many new exclusions.

Comment 11

[Jesse Ruderman]

The bugs that force me to exclude the most tests are:

* Bug 162063 - Tables
* Bug 409089 - -moz-box
* Bug 409125 - MathML
* Bug 229915 - CSS + combinator
* Bug 373298 - :-moz-first-node 
* Bug 145419 - dynamically added ::first-letter and ::first-line rules

Comment 12

[Jesse Ruderman]

After finding dozens of harmless layout inconsistencies, refdyn finally found a potential security hole!  Bug 421234 causes a random character to appear, and I'm guessing it comes from uninitialized memory.

Comment 13

[Jesse Ruderman]

Attachment: refdyn.zip

Comment 14

[Jesse Ruderman]

Attachment: refdyn.zip (snapshot from 2009-05-15)

Refdyn has only found two security holes among about a hundred total bugs.  I'd like to make it public eventually, but one of those bugs (bug 467323) isn't fixed on 3.0.x, so it might be a while.

Comment 15

[Jesse Ruderman]

I've revived this tool by making it part of the DOM fuzzer. Now it makes a number of random dynamic changes before checking that the resulting dynamic rendering matches the static rendering of the same DOM tree.

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:decoder, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 17

[Christian Holler (:decoder)]

The approach here sounds similar to what Layout Quick Check does (on top of our reftests), does that sound about right? Is there anything we can learn from this older bug for our deployment / use of LQIC?