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

Bug#927361: marked as done (unblock: node-superagent/0.20.0+dfsg-2)



Your message dated Thu, 18 Apr 2019 13:35:00 +0000
with message-id <02363626-ea6a-2f07-09bc-974c1050c6b0@thykier.net>
and subject line Re: Bug#927361: unblock: node-superagent/0.20.0+dfsg-2
has caused the Debian Bug report #927361,
regarding unblock: node-superagent/0.20.0+dfsg-2
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
927361: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=927361
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Please unblock package node-superagent

Hi all,

node-superagent is vulnerable to CVE-2017-16129 (tag high). Unfortunatly
no corresponding BTS has been filled, and I didn't see this before. I
imported the upstreal patch and updated the package and enable some
upstream tests (not all due to missing dependencies). Here is the full
changes:
  * Declare compliance with policy 4.3.0
  * Change section to javascript
  * Change priority to optional
  * Patch to fix ZIP bomb attacks (Closes: CVE-2017-16129)
  * Fix tests for Node.js >= 10
  * Enable some upstream tests using pkg-js-tools
  * Fix VCS fields
  * Fix debian/copyright format URL
  * Add upstream/metadata

Reverse dependencies:
 node-superagent
  +-(build)-> node-multiparty
  +-> node-supertest
      |
   (build dependencies)
      |
      V
      +- node-body-parser
      +- node-compression
      +- node-connect
      +- node-connect-timeout
      +- node-cookie-parser
      +- node-errorhandler
      +- node-finalhandler
      +- node-on-headers
      +- node-response-time
      +- node-send
      +- node-serve-index
      +- node-serve-static
      +- node-vary
      +- node-vhost

Change on installed files just adds a limit to response body content
(CVE-2017-16129.diff patch). The rest of this debdiff updates debian/*
files and enable tests (build & autopkgtest).

I think this CVE should be considered as RC bug, so I think it is needed
and low risky to unblock node-superagent.

Cheers,
Xavier
diff --git a/debian/changelog b/debian/changelog
index 0df52d2..e52f880 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,18 @@
+node-superagent (0.20.0+dfsg-2) unstable; urgency=medium
+
+  * Team upload
+  * Declare compliance with policy 4.3.0
+  * Change section to javascript
+  * Change priority to optional
+  * Patch to fix ZIP bomb attacks (Closes: CVE-2017-16129)
+  * Fix tests for Node.js >= 10
+  * Enable some upstream tests using pkg-js-tools
+  * Fix VCS fields
+  * Fix debian/copyright format URL
+  * Add upstream/metadata
+
+ -- Xavier Guimard <yadd@debian.org>  Thu, 18 Apr 2019 14:22:09 +0200
+
 node-superagent (0.20.0+dfsg-1) unstable; urgency=medium
 
   * Imported Upstream version 0.20.0+dfsg
diff --git a/debian/control b/debian/control
index 8a9adb8..4207e63 100644
--- a/debian/control
+++ b/debian/control
@@ -1,17 +1,30 @@
 Source: node-superagent
-Section: web
-Priority: extra
+Section: javascript
+Priority: optional
 Maintainer: Debian Javascript Maintainers <pkg-javascript-devel@lists.alioth.debian.org>
 Uploaders: Leo Iannacone <l3on@ubuntu.com>
+Testsuite: autopkgtest-pkg-nodejs
 Build-Depends:
  debhelper (>= 8)
  , dh-buildinfo
+ , mocha
  , nodejs
-Standards-Version: 3.9.6
+ , node-cookiejar
+ , node-cookie-parser
+ , node-debug
+ , node-express
+ , node-extend
+ , node-formidable
+ , node-form-data
+ , node-methods
+ , node-mime
+ , node-qs
+ , node-should
+ , pkg-js-tools
+Standards-Version: 4.3.0
+Vcs-Browser: https://salsa.debian.org/js-team/node-superagent
+Vcs-Git: https://salsa.debian.org/js-team/node-superagent.git
 Homepage: https://github.com/visionmedia/superagent
-Vcs-Git: git://anonscm.debian.org/pkg-javascript/node-superagent.git
-Vcs-Browser: http://anonscm.debian.org/gitweb/?p=pkg-javascript/node-superagent.git
-XS-Testsuite: autopkgtest
 
 Package: node-superagent
 Architecture: all
diff --git a/debian/copyright b/debian/copyright
index c1b2e17..216a109 100644
--- a/debian/copyright
+++ b/debian/copyright
@@ -1,4 +1,4 @@
-Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
+Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
 Upstream-Name: superagent
 Upstream-Contact: https://github.com/visionmedia/superagent/issues
 Source: https://github.com/visionmedia/superagent
@@ -39,4 +39,3 @@ License: Expat
  ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
  CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  SOFTWARE.
-
diff --git a/debian/patches/CVE-2017-16129.diff b/debian/patches/CVE-2017-16129.diff
new file mode 100644
index 0000000..7fc56a9
--- /dev/null
+++ b/debian/patches/CVE-2017-16129.diff
@@ -0,0 +1,34 @@
+Description: Fix for CVE-2017-16129
+Author: Xavier Guimard <yadd@debian.org>
+Origin: https://github.com/visionmedia/superagent/commit/946e28dab08f2ab334753bf36a2fbc5110d17789
+Bug: https://security-tracker.debian.org/tracker/CVE-2017-16129
+Forwarded: https://github.com/visionmedia/superagent/commit/946e28dab08f2ab334753bf36a2fbc5110d17789
+Last-Update: 2019-04-18
+
+--- a/lib/node/index.js
++++ b/lib/node/index.js
+@@ -898,6 +898,24 @@
+     // explicit parser
+     if (parser) parse = parser;
+ 
++        if (buffer) {
++      // Protectiona against zip bombs and other nuisance
++      let responseBytesLeft = self._maxResponseSize || 200000000;
++      res.on('data', function(buf) {
++        responseBytesLeft -= buf.byteLength || buf.length;
++        if (responseBytesLeft < 0) {
++          // This will propagate through error event
++          const err = Error("Maximum response size reached");
++          err.code = "ETOOLARGE";
++          // Parsers aren't required to observe error event,
++          // so would incorrectly report success
++          parserHandlesEnd = false;
++          // Will emit error event
++          res.destroy(err);
++        }
++      });
++    }
++
+     // parse
+     if (parse) {
+       try {
diff --git a/debian/patches/close-test-servers.diff b/debian/patches/close-test-servers.diff
new file mode 100644
index 0000000..f43e2c0
--- /dev/null
+++ b/debian/patches/close-test-servers.diff
@@ -0,0 +1,212 @@
+Description: Fix for Node.js 10
+Author: Xavier Guimard <yadd@debian.org>
+Forwarded: not-needed
+Last-Update: 2019-04-18
+
+--- a/test/node/agency.js
++++ b/test/node/agency.js
+@@ -4,8 +4,10 @@
+   , assert = require('assert')
+   , should = require('should');
+ 
+-app.use(express.cookieParser());
+-app.use(express.session({ secret: 'secret' }));
++var cookieParser = require('cookie-parser');
++var session = require('express-session');
++app.use(cookieParser());
++app.use(session({ secret: 'secret' }));
+ 
+ app.post('/signin', function(req, res) {
+   req.session.user = 'hunter@hunterloftis.com';
+--- a/test/node/flags.js
++++ b/test/node/flags.js
+@@ -29,7 +29,7 @@
+   res.send(204);
+ });
+ 
+-app.listen(3004);
++var s = app.listen(3004);
+ 
+ describe('flags', function(){
+ 
+@@ -116,4 +116,10 @@
+       });
+     })
+   })
+-})
+\ No newline at end of file
++  describe('end test', function(){
++    it('should stop server', function(done){
++      s.close();
++      done();
++    });
++  });
++})
+--- a/test/node/image.js
++++ b/test/node/image.js
+@@ -19,7 +19,7 @@
+     res.end(img, 'binary');
+   });
+ 
+-  app.listen(3011);
++  var s = app.listen(3011);
+ 
+   describe('image/png', function(){
+     it('should parse the body', function(done){
+@@ -31,4 +31,10 @@
+       });
+     });
+   });
++  describe('end test', function(){
++    it('should stop server', function(done){
++      s.close();
++      done();
++    });
++  });
+ });
+--- a/test/node/inflate.js
++++ b/test/node/inflate.js
+@@ -15,7 +15,7 @@
+   var app = express()
+     , subject = 'some long long long long string';
+ 
+-  app.listen(3080);
++  s = app.listen(3080);
+ 
+   app.get('/binary', function(req, res){
+     zlib.deflate(subject, function (err, buf){
+@@ -37,7 +37,7 @@
+       request
+         .get('http://localhost:3080')
+         .end(function(res){
+-          res.should.have.status(200);
++          res.status.should.eql(200);
+           res.text.should.equal(subject);
+           res.headers['content-length'].should.be.below(subject.length);
+           done();
+@@ -49,7 +49,7 @@
+         request
+           .get('http://localhost:3080/binary')
+           .end(function(res){
+-            res.should.have.status(200);
++            res.status.should.eql(200);
+             res.headers['content-length'].should.be.below(subject.length);
+ 
+             res.on('data', function(chunk){
+@@ -61,4 +61,10 @@
+       })
+     })
+   });
++  describe('end test', function(){
++    it('should stop server', function(done){
++      s.close();
++      done();
++    });
++  });
+ }
+--- a/test/node/pipe.js
++++ b/test/node/pipe.js
+@@ -4,7 +4,8 @@
+   , app = express()
+   , fs = require('fs');
+ 
+-app.use(express.bodyParser());
++var bodyParser = require('body-parser')
++app.use(bodyParser.json());
+ 
+ app.post('/', function(req, res){
+   res.send(req.body);
+--- a/test/node/redirects-other-host.js
++++ b/test/node/redirects-other-host.js
+@@ -9,7 +9,7 @@
+   res.redirect('https://github.com/');
+ });
+ 
+-app.listen(3210);
++var s = app.listen(3210);
+ 
+ describe('request', function(){
+   describe('on redirect', function(){
+@@ -24,4 +24,10 @@
+         });
+     })
+   });
+-});
+\ No newline at end of file
++});
++describe('end test', function(){
++  it('should stop server', function(done){
++    s.close();
++    done();
++  });
++});
+--- a/test/node/response-readable-stream.js
++++ b/test/node/response-readable-stream.js
+@@ -8,7 +8,7 @@
+   fs.createReadStream('test/node/fixtures/user.json').pipe(res);
+ });
+ 
+-app.listen(3025);
++var s = app.listen(3025);
+ 
+ describe('response', function(){
+     it('should act as a readable stream', function(done){
+@@ -39,3 +39,9 @@
+       });
+     });
+ });
++describe('end test', function(){
++  it('should stop server', function(done){
++    s.close();
++    done();
++  });
++});
+--- a/test/node/timeout.js
++++ b/test/node/timeout.js
+@@ -11,7 +11,7 @@
+   }, ms);
+ });
+ 
+-app.listen(3009);
++var s = app.listen(3009);
+ 
+ describe('.timeout(ms)', function(){
+   describe('when timeout is exceeded', function(done){
+@@ -26,4 +26,10 @@
+       });
+     })
+   })
+-})
+\ No newline at end of file
++})
++describe('end test', function(){
++  it('should stop server', function(done){
++    s.close();
++    done();
++  });
++});
+--- a/test/server.js
++++ b/test/server.js
+@@ -15,8 +15,10 @@
+   req.pipe(res);
+ });
+ 
+-app.use(express.bodyParser());
+-app.use(express.cookieParser());
++var cookieParser = require('cookie-parser')
++var bodyParser = require('body-parser')
++app.use(bodyParser.json());
++app.use(cookieParser());
+ 
+ app.use(function(req, res, next){
+   res.cookie('name', 'tobi');
+@@ -132,7 +134,8 @@
+   res.send(req.cookies.name);
+ });
+ 
+-app.use(express.static(__dirname + '/../'));
++var serveStatic = require('serve-static')
++app.use(serveStatic(__dirname + '/../'));
+ 
+ var server = app.listen(process.env.ZUUL_PORT, function() {
+   //console.log('Test server listening on port %d', server.address().port);
diff --git a/debian/patches/series b/debian/patches/series
index c366f88..711a168 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,3 @@
 no_require_readable-stream.patch
+CVE-2017-16129.diff
+close-test-servers.diff
diff --git a/debian/rules b/debian/rules
index 5e2158d..e6bbb30 100755
--- a/debian/rules
+++ b/debian/rules
@@ -5,14 +5,9 @@
 #export DH_VERBOSE=1
 
 %:
-	dh $@
+	dh $@ --with nodejs
 
 override_dh_auto_build:
 
-# Tests require express 3.x, but in debian we have 4.x
-# So, do not run tests untill this issues is open:
-#  https://github.com/visionmedia/superagent/issues/390
-override_dh_auto_test:
-
 override_dh_installchangelogs:
 	dh_installchangelogs -k History.md
diff --git a/debian/tests/control b/debian/tests/control
deleted file mode 100644
index 5b473bc..0000000
--- a/debian/tests/control
+++ /dev/null
@@ -1,2 +0,0 @@
-Tests: require
-Depends: node-superagent
diff --git a/debian/tests/pkg-js/test b/debian/tests/pkg-js/test
new file mode 100644
index 0000000..7ba0e47
--- /dev/null
+++ b/debian/tests/pkg-js/test
@@ -0,0 +1,11 @@
+mocha --require should --timeout 20000 \
+	test/node/exports.js \
+	test/node/flags.js \
+	test/node/image.js \
+	test/node/incoming-multipart.js \
+	test/node/inflate.js \
+	test/node/multipart.js \
+	test/node/network-error.js \
+	test/node/response-readable-stream.js \
+	test/node/timeout.js \
+	test/node/utils.js
diff --git a/debian/tests/require b/debian/tests/require
deleted file mode 100644
index b0609bb..0000000
--- a/debian/tests/require
+++ /dev/null
@@ -1,3 +0,0 @@
-#!/bin/sh
-set -e
-nodejs -e "require('superagent');"
diff --git a/debian/upstream/metadata b/debian/upstream/metadata
new file mode 100644
index 0000000..f0dd284
--- /dev/null
+++ b/debian/upstream/metadata
@@ -0,0 +1,7 @@
+---
+Archive: GitHub
+Bug-Database: https://github.com/visionmedia/superagent/issues
+Contact: https://github.com/visionmedia/superagent/issues
+Name: superagent
+Repository: https://github.com/visionmedia/superagent.git
+Repository-Browse: https://github.com/visionmedia/superagent

--- End Message ---
--- Begin Message ---
Xavier Guimard:
> Package: release.debian.org
> Severity: normal
> User: release.debian.org@packages.debian.org
> Usertags: unblock
> 
> Please unblock package node-superagent
> 
> Hi all,
> 
> node-superagent is vulnerable to CVE-2017-16129 (tag high). Unfortunatly
> no corresponding BTS has been filled, and I didn't see this before. I
> imported the upstreal patch and updated the package and enable some
> upstream tests (not all due to missing dependencies). Here is the full
> changes:
>   * Declare compliance with policy 4.3.0
>   * Change section to javascript
>   * Change priority to optional
>   * Patch to fix ZIP bomb attacks (Closes: CVE-2017-16129)
>   * Fix tests for Node.js >= 10
>   * Enable some upstream tests using pkg-js-tools
>   * Fix VCS fields
>   * Fix debian/copyright format URL
>   * Add upstream/metadata
> 
> Reverse dependencies:
>  node-superagent
>   +-(build)-> node-multiparty
>   +-> node-supertest
>       |
>    (build dependencies)
>       |
>       V
>       +- node-body-parser
>       +- node-compression
>       +- node-connect
>       +- node-connect-timeout
>       +- node-cookie-parser
>       +- node-errorhandler
>       +- node-finalhandler
>       +- node-on-headers
>       +- node-response-time
>       +- node-send
>       +- node-serve-index
>       +- node-serve-static
>       +- node-vary
>       +- node-vhost
> 
> Change on installed files just adds a limit to response body content
> (CVE-2017-16129.diff patch). The rest of this debdiff updates debian/*
> files and enable tests (build & autopkgtest).
> 
> I think this CVE should be considered as RC bug, so I think it is needed
> and low risky to unblock node-superagent.
> 
> Cheers,
> Xavier
> 

Unblocked, thanks.
~Niels

--- End Message ---

Reply to: