[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Security concerns with minified javascript code



 ❦  4 septembre 2015 08:29 +0100, Neil Williams <codehelp@debian.org> :

>> > 2. Many javascript packages cannot be build from source with the
>> > tools in main.
>> 
>> This is the one I don't agree. For me, pre-minification source is
>> source code. If you show the pre-minification version of jQuery, many
>> people will believe this is a valid source code (original comments,
>> variable names and indentation are here).
>
> ... except that what you are referring to there is the "source" as it
> appears as an unminified JS file on the filesystem after the upstream
> code is packaged by Debian. The JQuery upstream is a set of directives
> to include other snippet files which are then further processed into
> both the unminified JS that looks like source code and the minified JS
> which tends to get used by applications. The "source code" that
> everyone talks about is actually not edited by anyone, least of all
> upstream, it not used by applications (which embed the .min.js) and it
> is not convertible to the source for modification used by JQuery
> upstream, so patches to that are of no use to fixing security bugs in
> JQuery as packaged in Debian.

The pre-minification source code is so close of the original code that
you will have no trouble to apply any security fix to the
pre-minification source code. Let me take a random source file in the
original source:

 https://github.com/jquery/jquery/blob/2.1-stable/src/queue/delay.js

And the pre-minification source code:

 https://github.com/jquery/jquery-ui/blob/master/external/jquery-2.1.3/jquery.js#L6894-L6906

> JQuery in Debian *does* build from upstream source using only tools
> available in Debian main. What it builds from is not usable to
> applications, but if a bug is reported in the minified or unminified
> JS, the maintainer (together with the JQuery upstream) can prepare a
> patch to the source code that is built in Debian to produce a change in
> the minified and unminified *generated files* for other applications to
> use.

With the only problem that jQuery as shipped by Debian is currently 1.x
while some applications now require 2.x. And jQuery was just an example.

> Patching an unminified JS file as it exists on the filesystem from
> packaging doesn't actually help anyone, except possibly in debugging the
> security hole. Even then, one of the more serious problems with
> minification is that it isn't just a process of removing comments and
> whitespace, the minifier can inject bugs which are not present in the
> unminified version. Using a different minifier can introduce *different*
> *unknown* bugs that only affect the version patched to "improve
> security".

Exactly like a C compiler which can introduce bugs on its own. And
different C compilers have different bugs. Again:
https://www.alchemistowl.org/pocorgtfo/pocorgtfo08.pdf, 8:3.

> We really need to think of minifiers as transformational - these are
> compilers in many ways.

OK with that.

> For those upstreams who have to embed JS not available in Debian, then
> the upstream code often cannot be processed in Debian, neither can the
> unminified JS be minified into the same .min.js file as embedded due
> to a lack of tools in Debian.  In that sense, the current lintian
> requirements are a hindrance and a mess because they require the
> embedding upstream to include "source code" which cannot be converted
> to the actual running code using tools in main in a reproducible
> manner. Using a different minifier results in an untested and
> unverifiable .min.js with no assurance that new bugs have not been
> introduced.

The JS ecosystem is mature enough to know the problems that may arise
with minifiers and work around them. While using yui-compressor is a bit
risky due to its low use, using uglifyjs should be fine. The
pre-minified JS files usually are prepared to be minified (for example,
when using dependency injections, variables are usually converted to
strings to keep their original names during minification, example
available on request).

> This thread isn't principally about how JS packages already in main
> actually get built and patched, this is about how upstreams who need to
> embed JS not available in Debian can support security fixes to
> javascript. Patches which produce a third form of the code which
> produces a diff to the buggy version that is massively larger than the
> security fix itself will not find favour with the release team or
> security team during a release freeze.

Hopefully, this is not the case. An upstream patch can be transformed
into a patch which can be applied to a pre-minification version
"easily". And the result should be of the same size.

There is not a lot of CVE available to use as examples (I think mostly
because bugs get rarely classified as security bugs in this
ecosystem). After a quick search, the most recent one I am able to find
is CVE-2011-4969. The patch is here:

 https://github.com/jquery/jquery/commit/db9e023e62c1ff5d8f21ed9868ab6878da2005e9

#+begin_src diff
diff --git a/src/core.js b/src/core.js
index 694f884..0b99b74 100644
--- a/src/core.js
+++ b/src/core.js
@@ -16,8 +16,8 @@ var jQuery = function( selector, context ) {
 	rootjQuery,
 
 	// A simple way to check for HTML strings or ID strings
-	// (both of which we optimize for)
-	quickExpr = /^(?:[^<]*(<[\w\W]+>)[^>]*$|#([\w\-]*)$)/,
+	// Prioritize #id over <tag> to avoid XSS via location.hash (#9521)
+	quickExpr = /^(?:[^#<]*(<[\w\W]+>)[^>]*$|#([\w\-]*)$)/,
 
 	// Check if a string has a non-whitespace character in it
 	rnotwhite = /\S/,
#+end_src

Let me download a pre-minification version of jQuery 1.6.2:

 http://code.jquery.com/jquery-1.6.2.js

Patch applies directly:

#v+
$ patch --dry-run jquery-1.6.2.js < cve-2011-4969.patch
checking file jquery-1.6.2.js
Hunk #1 succeeded at 37 (offset 21 lines).
#v-

This will become more difficult with jQuery 3.x (due to ES6 to ES5
transformation). But manual adaptation should stay straightfoward
("transpilation" only does local transformation).

That's why I consider this pre-minification version as valid source
code.

>> > 3. Software that cannot be build from source with the tools in main
>> >    must not go in main but into contrib
>> 
>> I agree. And doing from pre-minification source to minified source is
>> possible with the tools in main (uglifyjs or yui-compressor).
>
> Not in a way that reproduces the same .min.js as the original source -
> unless that is from a package already in Debian and the same minifier
> is used (with the same options).

Getting the same min.js is not a goal (we don't try to get the same
binaries than upstream from source code in any other language, for
example in Go where upstreams like to provide binaries). Not adding bugs
would be nice, yes, but the situation is improving upstream (see above).
-- 
Make sure comments and code agree.
            - The Elements of Programming Style (Kernighan & Plauger)

Attachment: signature.asc
Description: PGP signature


Reply to: