From 337e87ae5fcea3667864197c65dc758517fcde06 Mon Sep 17 00:00:00 2001 From: Douglas Christopher Wilson Date: Mon, 20 Jan 2020 20:48:22 -0500 Subject: [PATCH] Add localInfile option to control LOAD DATA LOCAL INFILE closes #2185 --- Changes.md | 1 + Readme.md | 4 +- lib/ConnectionConfig.js | 8 ++++ lib/protocol/packets/EmptyPacket.js | 3 ++ .../packets/LocalInfileRequestPacket.js | 21 ++++++++++ lib/protocol/packets/ResultSetHeaderPacket.js | 11 ----- lib/protocol/packets/index.js | 1 + lib/protocol/sequences/Query.js | 32 ++++++++++----- test/FakeServer.js | 10 +++-- .../test-load-data-infile-disable.js | 38 +++++++++++++++++ .../connection/test-load-data-infile.js | 3 +- .../test-load-data-infile-disable.js | 41 +++++++++++++++++++ 12 files changed, 146 insertions(+), 27 deletions(-) create mode 100644 lib/protocol/packets/LocalInfileRequestPacket.js create mode 100644 test/integration/connection/test-load-data-infile-disable.js create mode 100644 test/unit/connection/test-load-data-infile-disable.js diff --git a/Changes.md b/Changes.md index 6d1716d1c..49af77f97 100644 --- a/Changes.md +++ b/Changes.md @@ -6,6 +6,7 @@ you spot any mistakes. ## HEAD +* Add `localInfile` option to control `LOAD DATA LOCAL INFILE` * Add new error codes up to MySQL 5.7.29 * Fix early detection of bad callback to `connection.query` * Support Node.js 12.x #2211 diff --git a/Readme.md b/Readme.md index dc71b8b58..51bd6321e 100644 --- a/Readme.md +++ b/Readme.md @@ -240,6 +240,7 @@ issue [#501](https://github.com/mysqljs/mysql/issues/501). (Default: `false`) * `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 @@ -323,7 +324,8 @@ The following flags are available: - `INTERACTIVE` - Indicates to the MySQL server this is an "interactive" client. This will use the interactive timeouts on the MySQL server and report as interactive in the process list. (Default off) -- `LOCAL_FILES` - Can use `LOAD DATA LOCAL`. (Default on) +- `LOCAL_FILES` - Can use `LOAD DATA LOCAL`. This flag is controlled by the connection + option `localInfile`. (Default on) - `LONG_FLAG` - Longer flags in Protocol::ColumnDefinition320. (Default on) - `LONG_PASSWORD` - Use the improved version of Old Password Authentication. (Default on) diff --git a/lib/ConnectionConfig.js b/lib/ConnectionConfig.js index 147aa0abb..06f4399c5 100644 --- a/lib/ConnectionConfig.js +++ b/lib/ConnectionConfig.js @@ -33,6 +33,9 @@ function ConnectionConfig(options) { 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 @@ ConnectionConfig.getDefaultFlags = function getDefaultFlags(options) { '+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'); diff --git a/lib/protocol/packets/EmptyPacket.js b/lib/protocol/packets/EmptyPacket.js index 3d3410659..27dd68604 100644 --- a/lib/protocol/packets/EmptyPacket.js +++ b/lib/protocol/packets/EmptyPacket.js @@ -2,5 +2,8 @@ module.exports = EmptyPacket; function EmptyPacket() { } +EmptyPacket.prototype.parse = function parse() { +}; + EmptyPacket.prototype.write = function write() { }; diff --git a/lib/protocol/packets/LocalInfileRequestPacket.js b/lib/protocol/packets/LocalInfileRequestPacket.js new file mode 100644 index 000000000..b1f68bab4 --- /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); +}; diff --git a/lib/protocol/packets/ResultSetHeaderPacket.js b/lib/protocol/packets/ResultSetHeaderPacket.js index 25b800222..a097ea1f2 100644 --- a/lib/protocol/packets/ResultSetHeaderPacket.js +++ b/lib/protocol/packets/ResultSetHeaderPacket.js @@ -3,23 +3,12 @@ function ResultSetHeaderPacket(options) { 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); - } }; diff --git a/lib/protocol/packets/index.js b/lib/protocol/packets/index.js index f36b87bfb..5e9352457 100644 --- a/lib/protocol/packets/index.js +++ b/lib/protocol/packets/index.js @@ -13,6 +13,7 @@ exports.Field = require('./Field'); 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'); diff --git a/lib/protocol/sequences/Query.js b/lib/protocol/sequences/Query.js index dd340abf7..b7632959b 100644 --- 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('readable-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 @@ Query.prototype.determinePacket = function determinePacket(byte, parser) { 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 @@ Query.prototype['ErrorPacket'] = function(packet) { 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); }; diff --git a/test/FakeServer.js b/test/FakeServer.js index f7a367518..28f67f257 100644 --- a/test/FakeServer.js +++ b/test/FakeServer.js @@ -277,8 +277,8 @@ FakeConnection.prototype._handleQueryPacket = function _handleQueryPacket(packet 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._parsePacket = function() { } }; -FakeConnection.prototype._determinePacket = function _determinePacket() { +FakeConnection.prototype._determinePacket = function _determinePacket(packetHeader) { if (this._expectedNextPacket) { var Packet = this._expectedNextPacket; @@ -353,6 +353,10 @@ FakeConnection.prototype._determinePacket = function _determinePacket() { return Packet; } + if (packetHeader.length === 0) { + return Packets.EmptyPacket; + } + var firstByte = this._parser.peak(); switch (firstByte) { case 0x01: return Packets.ComQuitPacket; diff --git a/test/integration/connection/test-load-data-infile-disable.js b/test/integration/connection/test-load-data-infile-disable.js new file mode 100644 index 000000000..89cdf4aaf --- /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); +}); diff --git a/test/integration/connection/test-load-data-infile.js b/test/integration/connection/test-load-data-infile.js index 4d5cc4285..ef5a72a3f 100644 --- a/test/integration/connection/test-load-data-infile.js +++ b/test/integration/connection/test-load-data-infile.js @@ -42,9 +42,10 @@ common.getTestConnection(function (err, connection) { 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); diff --git a/test/unit/connection/test-load-data-infile-disable.js b/test/unit/connection/test-load-data-infile-disable.js new file mode 100644 index 000000000..03d2be114 --- /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(); +});