benadida / helios-server

Helios server
http://heliosvoting.org
Apache License 2.0
731 stars 349 forks source link

Upgrade jQuery in Heliosbooth and Heliosverifier #395

Open laoumh opened 1 month ago

laoumh commented 1 month ago

Context

The main aim of this PR is to upgrade jQuery to the latest version, in order to mitigate known vulnerabilities associated with older versions: CVE-2015-9251, CVE-2020-11022 and CVE-2020-11023.

Since each app (sub module) has it's own set of JS libraries, I only tackled heliosbooth and heliosverifier for now. The Voting Booth was chosen first as it is the largest surface of interaction with the system (there are more people voting then administering election).

Documentation links were added to commit messages wherever pertinent.

Major changes

Removal of deprecated JS libraries

Since this PR is security-oriented, beyond upgrading jQuery I also tried to remove technical debt whenever possible, using current Browser API alternatives, that are now widely available. Hence, the following jQuery plugins were removed:

Switch from jTemplates to Underscore.js templates

By removing jTemplates, we need another way to render the booth templates. Instead of adding a new library to the project, I just used the template functionality available on Underscore.js, which is simple but sufficient for this use case. Underscore.js have three types of delimiters:

We can use this to do logic control and loops. So, for instance, instead of jTemplates'

{#foreach $T.question.answers as answer}
...
{#/for}

we can substitute it by

<% for (const answer of question.answers) { %>
...
<% } %> // end for block

I believe that, in a sense, this is easier to maintain because it's just pure JS, instead of having its own syntax like jTemplates.

Another (better?) solution would have been use Django templates, but that would have been a much bigger change: adding routing and view, changing the client logic of how to show and hide each voting booth step etc.

Finally, Underscore.js was also updated to the latest version, with no breaking changes.

JS libraries compression

I could install but not run uglify. It seems this package is not maintained and it now breaks. But, inspecting the package code, it seems uglify uses grunt-contrib-uglify, which in turn uses uglifyjs. So, I changed heliosbooth build command to use uglifyjs with compression and mangle options (details in the commit message).

Tests

The following tests were performed:

These tests were done on latest versions of Firefox, Chromium and Microsoft Edge.

A Few Considerations

This is a sensitive PR. I tried to test the changes as thoroughly as possible, but edge cases might have escaped me. For instance, could there be some portion of jscrypto that remained uncovered and might break? Please let me know if additional tests are needed.

Here are a few considerations after this upgrade process.

Duplicate JS libraries

A major difficulty with this upgrade was duplication of libraries (is it intentional or a result of bringing each sub module into the main project?). For example, there are five instances of jQuery through out the project:

./helios/media/js/jquery-1.4.2.min.js
./helios/media/helios/jquery-1.2.2.min.js
./heliosverifier/js/jquery-1.2.2.min.js
./server_ui/media/foundation/js/vendor/jquery.js
./heliosbooth/js/jquery-1.2.6.min.js

I believe it would have been possible to upgrade the whole project if there was a single source for statics.

Build

From the perspective of auditing the Voting Booth, wouldn't it be better to not mangle and compress JS libraries? The difference between compressed and uncompressed JS is 45.6 kB. Minifying (without mangling or compression) just jscrypto library reduces this difference to just 10.4 kB.

Also, boothworker-single.js loads jscrypto scripts anyway, so if we imported them directly on vote.html, we could leverage the browser's cache.

Small improvement loading templates

jTemplate's .setTemplateURL() method fetched templates in a synchronous fashion. The current solution fetches them asynchronously, in parallel, reducing load time:

jTemplate

native fetch

This change also solved a browser warning: Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience.