Compatibility issue with Waterfox 56.0.4

[JulioJu]

Issue type:

• Version compatibility

Version:

Version https://github.com/vimperator/vimperator-labs/tree/ff56-fixes

With Waterfox 56.0.4 (no problems with Waterfox 56.0.3)

Description:

When Vimperator is first started, in the command line we have the error message with red background
TypeError: this._divNodes.noCompletions is undefined.

I notice than we can't « follow hint ». Hint doesn't appear. Completion doesn't work too.

I've tested with the command $ waterfox -no-remote -P <fresh profile> -vimperator "+u NONE"

Expected behavior:

1. Functional « Hint mode »
2. Completion in command-line

Steps to reproduce:

1. Compile https://github.com/vimperator/vimperator-labs/tree/ff56-fixes, and Install vimperator on Waterfox
2. Restart Waterfox.
3. Open a google page with google (T google). At the first restart we see the message TypeError: this._divNodes.noCompletions is undefined
4. When we type on « f » key to have hints, we see 30 ms this message. No « hint » appear: we can't follow links.

Notes

I noticed than Vimperator team no supports officially Waterfox. But as you said on the README Vimperator could be installed on Waterfox, so maybe could you check quickly and correct this if it's simple ? Please could you do that ? If you believe it's a Waterfox bug, I could post an issue on https://github.com/MrAlex94/Waterfox/issues

Temporally, I've deleted line 1865 in the file comment/content/commandline.js (https://github.com/vimperator/vimperator-labs/blob/ff56-fixes/common/content/commandline.js. It resolves the problem with the hints, but sometimes there are some others bugs (I can't add spaces in the command line, and sometimes the scroll with keys bug), but it's lesser evil.

Please do not close this issue immediately. If it's complicated to fix this issue, maybe I could try to remove some features ? Maybe the completion ?

Thanks a lot in advance for your answer !

[devkev]

I noticed this also. The Waterfox Changelog lists only:

• Mozilla Foundation Security Advisory Patches:
  • 2018-02
  • 2018-05
• Update default search for some platforms. Feel free to use it or Ecosia, whichever you prefer to support Waterfox!

On perusing the contents of these security patches, the one that concerns me most is 2018-05, "Arbitrary code execution through unsanitized browser UI" (bugzilla 1432966). I worry that Vimperator is doing something which falls foul of some recently added sanitisation. However, I haven't yet looked into it in any real detail.

[devkev]

Okay, I had a quick look at this. The patch seems to only affect values set via innerHTML. Vimperator only has 2 places where it uses innerHTML, and both are to set it to an empty value. I confirmed that changing those to use the new setUnsafeInnerHTML() function, doesn't make the problem go away.

So it's either some other API that indirectly sets innerHTML, or it's something else entirely.

[JulioJu]

Thanks for your quick answer @devkev ;-) !

So actually, the only workaround is to revert BrowserWorks/waterfox@d7f689c, then compile Waterfox.

As you, I've tried to change innerHTML = "" by unsafeSetInnerHTML("") in common/content/commandline.js at the line 335 and at the line 1872 without any reslut.

I've posted an issue on the Waterfox project. See BrowserWorks/waterfox#428.

[JulioJu]

But the problem is that 2018-05 is marked as critical ! But it says that « This issue did not affect Firefox for Android or Firefox 52 ESR. ». Do you know if it impacts Firefox 56 ?

[devkev]

Thanks for going to the effort of bisecting to confirm that this is the problem!

One thing I just noticed is that I incorrectly used setUnsafeInnerHTML() in my testing, rather than the actual name which is unsafeSetInnerHTML() - which means my test was invalid. Are you sure that you used the right function name, and didn't just copy my mistake?

If so, then this means that the problem is that innerHTML is being called/used somewhere else (indirectly) by Vimperator. I have no idea where that might be.

I also notice that the patch makes reference to an allowUnsafeHTML attribute, which Vimperator might be able to be set on appropriate DOM document, eg. in the FF devtools browser.js. It ominiously says not to use this attribute; but presumably if Vimperator was to opt-in and set it to true, then the sanitisation protections would apply to everywhere in the browser except those parts of Vimperator which need it, and FF devtools. (The patch also says that there are other alternatives to using unsafeSetInnerHTML(), but annoyingly doesn't even hint at what they might be.)

But the problem is that 2018-05 is marked as critical ! But it says that « This issue did not affect Firefox for Android or Firefox 52 ESR. ». Do you know if it impacts Firefox 56 ?

My guess is that either FF56 has the affected code, or else it impacts Waterfox because Waterfox has backported the affected code from FF57/58. Either way, it would be extremely good to find a solution that doesn't involve reverting this critical patch. I think the next step is to try to understand and isolate the code in Vimperator which is being affected by this sanitisation.

[JulioJu]

Bisecting was long ! 100 % CPU used during more than one hour for each compilation ! But I believe it could be possible to use incremental compilation to save lot of times ;-). I would like to make a script to automatically bisect during the night, but Waterfox doesn't print Vimperator errors on the console, contrary to previous versions of Firefox. I don't know why, it's annoying.

I've checked. I've tested again. And I confirm than I've replaced innerHTML by unsafeSetInnerHTML() ;-).

After this I've performed a new test according to your last comment @devkev :

• replaced the line 335 by

doc.body.allowUnsafeHTML = true;
doc.body.unsafeSetInnerHTML("");

• replaced the line 1872 by clear: function clear() { this.setItems(); this._doc.body.allowUnsafeHTML = true ; this._doc.body.unsafeSetInnerHTML(""); },
• test in a new profile.
• ==> no result. Same problem as describe below.

I don't know how to debug a XUL plugin. We can't use about:debugging or web-ext (and I know it's normal ;-)).

[JulioJu]

I've tried also with document.allowUnsafeHTML = true in opposite to the example in Bugzilla/1433871. But doesn't work too (anyway, I guess it would be not secure).

In my custom version of Waterfox with BrowserWorks/waterfox@d7f689c reverted, now I use only unsafeSetInnerHTML('') instead of innerHTML and all work fine ! But as you said @devkev it's probably not secure because BrowserWorks/waterfox@d7f689c is reverted.

[JulioJu]

I've tested to replace innerHTML by somting like jfiijefjio, but Vimperator works well anyway in my custom version of Waterfox with BrowserWorks/waterfox@d7f689c reverted ! I've tested in a new profile.

So as you said @devkev « If so, then this means that the problem is that innerHTML is being called/used somewhere else (indirectly) by Vimperator. I have no idea where that might be. » ! But where and how ;-) ? It should have others DOM API who are linked to innerHTML ? But how and where ? And why they are not mentionning in bugzilla 1432966 ? Or there is a innerHTML is in a dependencie of Vimperator (but Vimperator hasn't dependencies ?) ?

[jpweytjens]

I've come across this issue as well. As a temporary workaround I've downloaded the latest ESR version of Firefox, being 52.6.0. Vimperator 3.16.0 works flawlessly with this version. Sadly, I can't help with the reason behind this observation.

[JulioJu]

Warning… Firefox ESR 52 will die soon !

Maybe, like Vimperator ?

Have you an idea to adapt Vimperator to BrowserWorks/waterfox#428 (comment) ?

Thanks a lot in advance !

[JulioJu]

@devkev, @u1z
Now Firefox ESR 52 is definitely dead… Have you a workaround to continue using Vimperator ? Personnaly, I've compiled my own version of Waterfox with $ git revert d7f689c984bf15259ae5c882ab7d36919f3bbda8. But it's not a good idea, this small commit is very important for security reasons.

Have you an idea if somebody can help us ?

Thanks in advance :-)

[mumuxme]

Same here (waterfox 56.2.5 on linux)...I have tested the following and it seems to work fine:

• enable allowUnsafeHTML while init ( line 1721 )

this._doc.allowUnsafeHTML = true;

• disable multi-process
• restart waterfox

(Hope any other better way)

[JulioJu]

@mumuxme thanks a lot ^^ ! I love you ! ;-) !

Maybe a new release of this could be added in https://github.com/vimperator/vimperator-labs/releases ? And a comment could be added in https://github.com/vimperator/vimperator-labs/blob/master/README.md ?

As it, we could still install and use the lovely Vimperator very easly ;-) !

Thanks a lot !