GNOME Bugzilla – Bug 751385
Hard-to-reproduce crash due to unescaped '%' chars in overview.html
Last modified: 2015-08-03 08:55:07 UTC
The contents of the overview.html file are used as a printf-style format string, which means that '%' are reserved characters that should be escaped as '%%' to appear in the output. Unfortunately the sizes expressed as percentages in the embedded CSS have unescaped '%' characters, leading the printf parser to try to read extra arguments that aren't really there. This doesn't always result in segfaults since '% 5e' makes the parser look for a double parameter with 5 digit precision and a space if the number is not negative: given that most calling convenctions put floating points parameters in their own specific registers this doesn't mess with the ordering of the real non-floating parameter and results in a random floating point value being read from a register and outputted in the middle of a CSS rule. This can be confirmed by looking with the inspector at the spurious floating value being printed in the middle of the CSS rule. The other bare '%' is followed instead by a ';' which is not a valid conversion type and gets outputted literally by printf. I've added some noisy comments in my patch to grab the attention of future developers as it's easy to overlook the issue as it's very subtle and it may cause some hard-to-reproduce crashes as in my case.
Created attachment 305921 [details] [review] overview: Escape bare '%' used in a printf-style template The contents of the overview.html file are used as a printf-style format string, which means that '%' are reserved characters that should be escaped as '%%' to appear in the output. Unfortunately the sizes expressed as percentages in the embedded CSS have unescaped '%' characters, leading the printf parser to try to read extra arguments that aren't really there. This didn't always result in segfaults since '% 5e' made the parser look for a double parameter with 5 digit precision and a space if the number is not negative: given that most calling convenctions put floating points parameters in their own specific registers this didn't mess with the ordering of the real non-floating parameter and resulted in a random floating point value being read from a register and outputted in the middle of a CSS rule. This can be confirmed by looking with the inspector at the spurious floating value being printed in the middle of the CSS rule. The other bare '%' was followed instead by a ';' which is not a valid conversion type and was outputted literally by printf. The noisy comments are there to grab the attention as it's easy to overlook the issue again when someone will edit the file in the future.
The fact that it's easy to overlook this makes me think that we shouldn't be doing it this way in the first place.
Yep, having the whole file to be a printf-style template can be surprising, and there's a good chance of colliding given how often '%' is used in CSS. One option could be to split the CSS out, it would be one more resource to load but I don't think it will cause a noticeable slowdown. Otherwise we could either edit the DOM from the C code or inject some JS data hand have some JavaScript code embedded in the page which picks the data up and edits the page contents accordingly.
Created attachment 308525 [details] [review] overview: Move the overview CSS to about.css and generate the HTML from the code
*** Bug 737994 has been marked as a duplicate of this bug. ***
It would be nice if you could get a patch out in a bugfix point release. While my Epiphany seldomly crashes from this bug these days, I never see the speed dial, only garbage characters.
Patch looks good, for gnome-3-16 as well as master.
Comment on attachment 308525 [details] [review] overview: Move the overview CSS to about.css and generate the HTML from the code Pushed to master and gnome-3-16 branch.