Package: release.debian.org Severity: normal Tags: buster User: release.debian....@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 <y...@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 <d...@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 <y...@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
