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

Re: Security concerns with minified javascript code

On Fri, 04 Sep 2015 10:07:13 +0200
Vincent Bernat <bernat@debian.org> wrote:

>  ❦  4 septembre 2015 08:29 +0100, Neil Williams
> <codehelp@debian.org> :
> > 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).

Actually, that is a great relief. Now this thread is getting somewhere
> > 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).

Are there supported tools which could prevent that difficulty becoming
a problem?

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

That indeed would support a usable security fix - thanks.

> >> > 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). -- 

OK. In that case, it might be worth embedding upstreams testing with
using uglifyjs on the unminified JS files included upstream and seeing
if there are (functional) bugs introduced by using that minifier. It
could also be worth asking lintian to change the meaning of the current
warnings so that merely accompanying the minified with the unminified
is not enough, that minification with something like uglifyjs should be
tested and that the maintainer should replace the upstream .min.js
with the output of that known minifier (in much the same way as
dh_autoreconf and for similar reasons). That neatly solves any DFSG
issues too.

What are the problems likely to be if Debian was to recommend/require
"minified javascript is not to be regarded as source code suitable for
supporting security fixes and needs to be rebuilt from unminified source
code or replaced with symlinks to minified javascript provided by other
packages, with dependencies added on those packages. When packages need
to minify embedded Javascript which is not already packaged in Debian,
only <foo> minifier should be used and any minified javascript files
included by upstream must be replaced using the output of <foo>

This excludes the issues of packaging of libjs* packages themselves
(where the same minifier as upstream is likely to be a significant
advantage in updating the package) but does provide a solution for the
actual security concerns of packaged minified Javascript. A handy tool
to do this would be useful as that would allow interested upstreams to
test the effects of such a minifier before the upload to Debian.



Neil Williams

Attachment: pgphucFXNWj3v.pgp
Description: OpenPGP digital signature

Reply to: