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

Bug#970307: marked as done (buster-pu: package node-mysql/2.16.0-1+deb10u1)



Your message dated Sat, 26 Sep 2020 11:36:30 +0100
with message-id <d50ba4de424290cd2840a09ef19950156fcf51ab.camel@adam-barratt.org.uk>
and subject line Closing bugs for fixes included in 10.6 point release
has caused the Debian Bug report #970307,
regarding buster-pu: package node-mysql/2.16.0-1+deb10u1
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.)


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

[ Reason ]
node-mysql is vulnerable to CVE-2019-14939 (#934712)

[ Impact ]
Default "LOAD DATA LOCAL INFILE" is too permissive

[ Tests ]
Sadly tests were not enabled in buster

[ Risks ]
Patch is exactly upstream one, seems low risky (it just adds a new
option)

[ Checklist ]
  [X] *all* changes are documented in the d/changelog
  [X] I reviewed all changes and I approve them
  [X] attach debdiff against the package in (old)stable
  [X] the issue is verified as fixed in unstable

[ Changes ]
Add a `localInfile` option that permits to change default LOCAL_FILES
flag
diff --git a/debian/changelog b/debian/changelog
index 8717915..a67cec7 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+node-mysql (2.16.0-1+deb10u1) buster; urgency=medium
+
+  * Team upload
+  * Add localInfile option to control LOAD DATA LOCAL INFILE
+    (Closes: #934712, CVE-2019-14939)
+
+ -- Xavier Guimard <yadd@debian.org>  Mon, 14 Sep 2020 15:57:57 +0200
+
 node-mysql (2.16.0-1) unstable; urgency=medium
 
   * Team upload
diff --git a/debian/patches/CVE-2019-14939.patch b/debian/patches/CVE-2019-14939.patch
new file mode 100644
index 0000000..8fe1dc7
--- /dev/null
+++ b/debian/patches/CVE-2019-14939.patch
@@ -0,0 +1,312 @@
+Description: Add localInfile option to control LOAD DATA LOCAL INFILE
+Author: Douglas Christopher Wilson <doug@somethingdoug.com>
+Origin: upstream, https://github.com/mysqljs/mysql/commit/337e87ae
+Bug: https://github.com/mysqljs/mysql/issues/2257
+Bug-Debian: https://bugs.debian.org/934712
+Forwarded: not-needed
+Reviewed-By: Xavier Guimard <yadd@debian.org>
+Last-Update: 2020-09-14
+
+--- a/Readme.md
++++ b/Readme.md
+@@ -229,6 +229,7 @@
+ * `trace`: Generates stack traces on `Error` to include call site of library
+    entrance ("long stack traces"). Slight performance penalty for most calls.
+    (Default: `true`)
++* `localInfile`: Allow `LOAD DATA INFILE` to use the `LOCAL` modifier. (Default: `true`)
+ * `multipleStatements`: Allow multiple mysql statements per query. Be careful
+   with this, it could increase the scope of SQL injection attacks. (Default: `false`)
+ * `flags`: List of connection flags to use other than the default ones. It is
+@@ -1362,7 +1363,8 @@
+ - `FOUND_ROWS` - Send the found rows instead of the affected rows as `affectedRows`.
+ - `IGNORE_SIGPIPE` - Old; no effect.
+ - `IGNORE_SPACE` - Let the parser ignore spaces before the `(` in queries.
+-- `LOCAL_FILES` - Can use `LOAD DATA LOCAL`.
++- `LOCAL_FILES` - Can use `LOAD DATA LOCAL`. This flag is controlled by the connection
++  option `localInfile`. (Default on)
+ - `LONG_FLAG`
+ - `LONG_PASSWORD` - Use the improved version of Old Password Authentication.
+ - `MULTI_RESULTS` - Can handle multiple resultsets for COM_QUERY.
+--- a/lib/ConnectionConfig.js
++++ b/lib/ConnectionConfig.js
+@@ -33,6 +33,9 @@
+   this.ssl                = (typeof options.ssl === 'string')
+     ? ConnectionConfig.getSSLProfile(options.ssl)
+     : (options.ssl || false);
++  this.localInfile        = (options.localInfile === undefined)
++    ? true
++    : options.localInfile;
+   this.multipleStatements = options.multipleStatements || false;
+   this.typeCast           = (options.typeCast === undefined)
+     ? true
+@@ -114,6 +117,11 @@
+     '+TRANSACTIONS'       // Expects status flags
+   ];
+ 
++  if (options && options.localInfile !== undefined && !options.localInfile) {
++    // Disable LOCAL modifier for LOAD DATA INFILE
++    defaultFlags.push('-LOCAL_FILES');
++  }
++
+   if (options && options.multipleStatements) {
+     // May send multiple statements per COM_QUERY and COM_STMT_PREPARE
+     defaultFlags.push('+MULTI_STATEMENTS');
+--- a/lib/protocol/packets/EmptyPacket.js
++++ b/lib/protocol/packets/EmptyPacket.js
+@@ -2,5 +2,8 @@
+ function EmptyPacket() {
+ }
+ 
++EmptyPacket.prototype.parse = function parse() {
++};
++
+ EmptyPacket.prototype.write = function write() {
+ };
+--- /dev/null
++++ b/lib/protocol/packets/LocalInfileRequestPacket.js
+@@ -0,0 +1,21 @@
++module.exports = LocalInfileRequestPacket;
++function LocalInfileRequestPacket(options) {
++  options = options || {};
++
++  this.filename = options.filename;
++}
++
++LocalInfileRequestPacket.prototype.parse = function parse(parser) {
++  if (parser.parseLengthCodedNumber() !== null) {
++    var err  = new TypeError('Received invalid field length');
++    err.code = 'PARSER_INVALID_FIELD_LENGTH';
++    throw err;
++  }
++
++  this.filename = parser.parsePacketTerminatedString();
++};
++
++LocalInfileRequestPacket.prototype.write = function write(writer) {
++  writer.writeLengthCodedNumber(null);
++  writer.writeString(this.filename);
++};
+--- a/lib/protocol/packets/ResultSetHeaderPacket.js
++++ b/lib/protocol/packets/ResultSetHeaderPacket.js
+@@ -3,23 +3,12 @@
+   options = options || {};
+ 
+   this.fieldCount = options.fieldCount;
+-  this.extra      = options.extra;
+ }
+ 
+ ResultSetHeaderPacket.prototype.parse = function(parser) {
+   this.fieldCount = parser.parseLengthCodedNumber();
+-
+-  if (parser.reachedPacketEnd()) return;
+-
+-  this.extra = (this.fieldCount === null)
+-    ? parser.parsePacketTerminatedString()
+-    : parser.parseLengthCodedNumber();
+ };
+ 
+ ResultSetHeaderPacket.prototype.write = function(writer) {
+   writer.writeLengthCodedNumber(this.fieldCount);
+-
+-  if (this.extra !== undefined) {
+-    writer.writeLengthCodedNumber(this.extra);
+-  }
+ };
+--- a/lib/protocol/packets/index.js
++++ b/lib/protocol/packets/index.js
+@@ -13,6 +13,7 @@
+ exports.FieldPacket = require('./FieldPacket');
+ exports.HandshakeInitializationPacket = require('./HandshakeInitializationPacket');
+ exports.LocalDataFilePacket = require('./LocalDataFilePacket');
++exports.LocalInfileRequestPacket = require('./LocalInfileRequestPacket');
+ exports.OkPacket = require('./OkPacket');
+ exports.OldPasswordPacket = require('./OldPasswordPacket');
+ exports.ResultSetHeaderPacket = require('./ResultSetHeaderPacket');
+--- a/lib/protocol/sequences/Query.js
++++ b/lib/protocol/sequences/Query.js
+@@ -1,10 +1,11 @@
+-var Sequence     = require('./Sequence');
+-var Util         = require('util');
+-var Packets      = require('../packets');
+-var ResultSet    = require('../ResultSet');
+-var ServerStatus = require('../constants/server_status');
+-var fs           = require('fs');
+-var Readable     = require('stream');
++var ClientConstants = require('../constants/client');
++var fs              = require('fs');
++var Packets         = require('../packets');
++var ResultSet       = require('../ResultSet');
++var Sequence        = require('./Sequence');
++var ServerStatus    = require('../constants/server_status');
++var Readable        = require('readable-stream');
++var Util            = require('util');
+ 
+ module.exports = Query;
+ Util.inherits(Query, Sequence);
+@@ -35,6 +36,7 @@
+   if (!resultSet) {
+     switch (byte) {
+       case 0x00: return Packets.OkPacket;
++      case 0xfb: return Packets.LocalInfileRequestPacket;
+       case 0xff: return Packets.ErrorPacket;
+       default:   return Packets.ResultSetHeaderPacket;
+     }
+@@ -90,14 +92,22 @@
+   this.end(err, results, fields);
+ };
+ 
+-Query.prototype['ResultSetHeaderPacket'] = function(packet) {
+-  if (packet.fieldCount === null) {
+-    this._sendLocalDataFile(packet.extra);
++Query.prototype['LocalInfileRequestPacket'] = function(packet) {
++  if (this._connection.config.clientFlags & ClientConstants.CLIENT_LOCAL_FILES) {
++    this._sendLocalDataFile(packet.filename);
+   } else {
+-    this._resultSet = new ResultSet(packet);
++    this._loadError       = new Error('Load local files command is disabled');
++    this._loadError.code  = 'LOCAL_FILES_DISABLED';
++    this._loadError.fatal = false;
++
++    this.emit('packet', new Packets.EmptyPacket());
+   }
+ };
+ 
++Query.prototype['ResultSetHeaderPacket'] = function(packet) {
++  this._resultSet = new ResultSet(packet);
++};
++
+ Query.prototype['FieldPacket'] = function(packet) {
+   this._resultSet.fieldPackets.push(packet);
+ };
+--- a/test/FakeServer.js
++++ b/test/FakeServer.js
+@@ -277,8 +277,8 @@
+   this.error('Interrupted unknown query', Errors.ER_QUERY_INTERRUPTED);
+ };
+ 
+-FakeConnection.prototype._parsePacket = function() {
+-  var Packet = this._determinePacket();
++FakeConnection.prototype._parsePacket = function _parsePacket(packetHeader) {
++  var Packet = this._determinePacket(packetHeader);
+   var packet = new Packet({protocol41: true});
+ 
+   packet.parse(this._parser);
+@@ -338,7 +338,7 @@
+   }
+ };
+ 
+-FakeConnection.prototype._determinePacket = function _determinePacket() {
++FakeConnection.prototype._determinePacket = function _determinePacket(packetHeader) {
+   if (this._expectedNextPacket) {
+     var Packet = this._expectedNextPacket;
+ 
+@@ -353,6 +353,10 @@
+     return Packet;
+   }
+ 
++  if (packetHeader.length === 0) {
++    return Packets.EmptyPacket;
++  }
++
+   var firstByte = this._parser.peak();
+   switch (firstByte) {
+     case 0x01: return Packets.ComQuitPacket;
+--- /dev/null
++++ b/test/integration/connection/test-load-data-infile-disable.js
+@@ -0,0 +1,38 @@
++var assert = require('assert');
++var common = require('../../common');
++
++var path    = common.fixtures + '/data.csv';
++var table   = 'load_data_test';
++var newline = common.detectNewline(path);
++
++common.getTestConnection({localInfile: false}, function (err, connection) {
++  assert.ifError(err);
++
++  common.useTestDb(connection);
++
++  connection.query([
++    'CREATE TEMPORARY TABLE ?? (',
++    '`id` int(11) unsigned NOT NULL AUTO_INCREMENT,',
++    '`title` varchar(400),',
++    'PRIMARY KEY (`id`)',
++    ') ENGINE=InnoDB DEFAULT CHARSET=utf8'
++  ].join('\n'), [table], assert.ifError);
++
++  var sql =
++    'LOAD DATA LOCAL INFILE ? INTO TABLE ?? CHARACTER SET utf8 ' +
++    'FIELDS TERMINATED BY ? ' +
++    'LINES TERMINATED BY ? ' +
++    '(id, title)';
++
++  connection.query(sql, [path, table, ',', newline], function (err) {
++    assert.ok(err);
++    assert.equal(err.code, 'ER_NOT_ALLOWED_COMMAND');
++  });
++
++  connection.query('SELECT * FROM ??', [table], function (err, rows) {
++    assert.ifError(err);
++    assert.equal(rows.length, 0);
++  });
++
++  connection.end(assert.ifError);
++});
+--- a/test/integration/connection/test-load-data-infile.js
++++ b/test/integration/connection/test-load-data-infile.js
+@@ -42,9 +42,10 @@
+     assert.equal(rows[4].title, 'this is a long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long long string');
+   });
+ 
+-  connection.query(sql, [badPath, table, ',', newline], function (err) {
++  connection.query(sql, [badPath, table, ',', newline], function (err, result) {
+     assert.ok(err);
+     assert.equal(err.code, 'ENOENT');
++    assert.equal(result.affectedRows, 0);
+   });
+ 
+   connection.end(assert.ifError);
+--- /dev/null
++++ b/test/unit/connection/test-load-data-infile-disable.js
+@@ -0,0 +1,41 @@
++var assert     = require('assert');
++var common     = require('../../common');
++var connection = common.createConnection({port: common.fakeServerPort, localInfile: false});
++var server     = common.createFakeServer();
++
++server.listen(common.fakeServerPort, function (err) {
++  assert.ifError(err);
++
++  connection.query('LOAD DATA LOCAL INFILE ? INTO TABLE ??', ['data.csv', 'foo'], function (err, result) {
++    assert.ok(err);
++    assert.equal(err.code, 'LOCAL_FILES_DISABLED');
++    assert.ok(!err.fatal);
++    assert.equal(result.affectedRows, 0);
++
++    connection.destroy();
++    server.destroy();
++  });
++});
++
++server.on('connection', function(conn) {
++  conn.on('clientAuthentication', function (packet) {
++    if (packet.clientFlags & common.ClientConstants.LOCAL_FILES) {
++      conn.deny();
++    } else {
++      conn.ok();
++    }
++  });
++  conn.on('query', function (packet) {
++    if (packet.sql.indexOf('LOAD DATA LOCAL INFILE') === 0) {
++      conn.once('EmptyPacket', function () {
++        conn.ok();
++      });
++      this._sendPacket(new common.Packets.LocalInfileRequestPacket({
++        filename: common.fixtures + '/data.csv'
++      }));
++    } else {
++      this._handleQueryPacket(packet);
++    }
++  });
++  conn.handshake();
++});
diff --git a/debian/patches/series b/debian/patches/series
index d7678e3..793ae1f 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,2 @@
 readable_stream.patch
+CVE-2019-14939.patch

--- End Message ---
--- Begin Message ---
Package: release.debian.org
Version: 10.6

Hi,

Each of these bugs relates to an update that was included in today's
stable point release.

Regards,

Adam

--- End Message ---

Reply to: