Skip to content

Commit 74abf9e

Browse files
wellwelwelwellwelwel
authored
fix(security): improve supportBigNumbers and bigNumberStrings sanitization (#2572)
Fixes a potential RCE attack vulnerability reported by Vsevolod Kokorin (Slonser) of Solidlab
1 parent 8a818ce commit 74abf9e

File tree

4 files changed

+160
-24
lines changed

4 files changed

+160
-24
lines changed

‎lib/parsers/binary_parser.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@ for (const t in Types) {
1212
}
1313

1414
function readCodeFor(field, config, options, fieldNum) {
15-
const supportBigNumbers =
16-
options.supportBigNumbers || config.supportBigNumbers;
17-
const bigNumberStrings = options.bigNumberStrings || config.bigNumberStrings;
15+
const supportBigNumbers = Boolean(
16+
options.supportBigNumbers || config.supportBigNumbers,
17+
);
18+
const bigNumberStrings = Boolean(
19+
options.bigNumberStrings || config.bigNumberStrings,
20+
);
1821
const timezone = options.timezone || config.timezone;
1922
const dateStrings = options.dateStrings || config.dateStrings;
2023
const unsigned = field.flags & FieldFlags.UNSIGNED;

‎lib/parsers/text_parser.js‎

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@ for (const t in Types) {
1212
}
1313

1414
function readCodeFor(type, charset, encodingExpr, config, options) {
15-
const supportBigNumbers =
16-
options.supportBigNumbers || config.supportBigNumbers;
17-
const bigNumberStrings = options.bigNumberStrings || config.bigNumberStrings;
15+
const supportBigNumbers = Boolean(
16+
options.supportBigNumbers || config.supportBigNumbers,
17+
);
18+
const bigNumberStrings = Boolean(
19+
options.bigNumberStrings || config.bigNumberStrings,
20+
);
1821
const timezone = options.timezone || config.timezone;
1922
const dateStrings = options.dateStrings || config.dateStrings;
2023

@@ -85,22 +88,24 @@ function compile(fields, options, config) {
8588
db: field.schema,
8689
table: field.table,
8790
name: field.name,
88-
string: function(encoding = field.encoding) {
91+
string: function (encoding = field.encoding) {
8992
if (field.columnType === Types.JSON && encoding === field.encoding) {
9093
// Since for JSON columns mysql always returns charset 63 (BINARY),
9194
// we have to handle it according to JSON specs and use "utf8",
9295
// see https://github.com/sidorares/node-mysql2/issues/1661
93-
console.warn(`typeCast: JSON column "${field.name}" is interpreted as BINARY by default, recommended to manually set utf8 encoding: \`field.string("utf8")\``);
96+
console.warn(
97+
`typeCast: JSON column "${field.name}" is interpreted as BINARY by default, recommended to manually set utf8 encoding: \`field.string("utf8")\``,
98+
);
9499
}
95100

96101
return _this.packet.readLengthCodedString(encoding);
97102
},
98-
buffer: function() {
103+
buffer: function () {
99104
return _this.packet.readLengthCodedBuffer();
100105
},
101-
geometry: function() {
106+
geometry: function () {
102107
return _this.packet.parseGeometryValue();
103-
}
108+
},
104109
};
105110
}
106111

@@ -109,9 +114,7 @@ function compile(fields, options, config) {
109114
/* eslint-disable no-trailing-spaces */
110115
/* eslint-disable no-spaced-func */
111116
/* eslint-disable no-unexpected-multiline */
112-
parserFn('(function () {')(
113-
'return class TextRow {'
114-
);
117+
parserFn('(function () {')('return class TextRow {');
115118

116119
// constructor method
117120
parserFn('constructor(fields) {');
@@ -127,22 +130,22 @@ function compile(fields, options, config) {
127130

128131
// next method
129132
parserFn('next(packet, fields, options) {');
130-
parserFn("this.packet = packet;");
133+
parserFn('this.packet = packet;');
131134
if (options.rowsAsArray) {
132135
parserFn(`const result = new Array(${fields.length});`);
133136
} else {
134-
parserFn("const result = {};");
137+
parserFn('const result = {};');
135138
}
136139

137140
const resultTables = {};
138141
let resultTablesArray = [];
139142

140143
if (options.nestTables === true) {
141-
for (let i=0; i < fields.length; i++) {
144+
for (let i = 0; i < fields.length; i++) {
142145
resultTables[fields[i].table] = 1;
143146
}
144147
resultTablesArray = Object.keys(resultTables);
145-
for (let i=0; i < resultTablesArray.length; i++) {
148+
for (let i = 0; i < resultTablesArray.length; i++) {
146149
parserFn(`result[${helpers.srcEscape(resultTablesArray[i])}] = {};`);
147150
}
148151
}
@@ -154,7 +157,7 @@ function compile(fields, options, config) {
154157
parserFn(`// ${fieldName}: ${typeNames[fields[i].columnType]}`);
155158
if (typeof options.nestTables === 'string') {
156159
lvalue = `result[${helpers.srcEscape(
157-
fields[i].table + options.nestTables + fields[i].name
160+
fields[i].table + options.nestTables + fields[i].name,
158161
)}]`;
159162
} else if (options.nestTables === true) {
160163
lvalue = `result[${helpers.srcEscape(fields[i].table)}][${fieldName}]`;
@@ -172,11 +175,13 @@ function compile(fields, options, config) {
172175
fields[i].characterSet,
173176
encodingExpr,
174177
config,
175-
options
178+
options,
176179
);
177180
if (typeof options.typeCast === 'function') {
178-
parserFn(`${lvalue} = options.typeCast(this.wrap${i}, function() { return ${readCode} });`);
179-
} else {
181+
parserFn(
182+
`${lvalue} = options.typeCast(this.wrap${i}, function() { return ${readCode} });`,
183+
);
184+
} else {
180185
parserFn(`${lvalue} = ${readCode};`);
181186
}
182187
}
@@ -193,11 +198,11 @@ function compile(fields, options, config) {
193198
if (config.debug) {
194199
helpers.printDebugWithCode(
195200
'Compiled text protocol row parser',
196-
parserFn.toString()
201+
parserFn.toString(),
197202
);
198203
}
199204
if (typeof options.typeCast === 'function') {
200-
return parserFn.toFunction({wrap});
205+
return parserFn.toFunction({ wrap });
201206
}
202207
return parserFn.toFunction();
203208
}

‎test/esm/unit/parsers/big-numbers-strings-sanitization.test.mjs‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { describe, test, assert } from 'poku';
2+
import { createConnection, describeOptions } from '../../../common.test.cjs';
3+
4+
const connection = createConnection().promise();
5+
6+
const sql = 'SELECT 9007199254740991+100 AS `total`';
7+
8+
describe('bigNumberStrings Sanitization', describeOptions);
9+
10+
Promise.all([
11+
test(async () => {
12+
const [results] = await connection.query({
13+
sql,
14+
supportBigNumbers: true,
15+
bigNumberStrings: true,
16+
});
17+
18+
assert.strictEqual(
19+
typeof results[0].total,
20+
'string',
21+
'Valid bigNumberStrings enabled',
22+
);
23+
}),
24+
test(async () => {
25+
const [results] = await connection.query({
26+
sql,
27+
supportBigNumbers: false,
28+
bigNumberStrings: false,
29+
});
30+
31+
assert.strictEqual(
32+
typeof results[0].total,
33+
'number',
34+
'Valid bigNumberStrings disabled',
35+
);
36+
}),
37+
38+
test(async () => {
39+
const [results] = await connection.query({
40+
sql,
41+
supportBigNumbers: 'text',
42+
bigNumberStrings: 'text',
43+
});
44+
45+
assert.strictEqual(
46+
typeof results[0].total,
47+
'string',
48+
'bigNumberStrings as a random string should be enabled',
49+
);
50+
}),
51+
test(async () => {
52+
const [results] = await connection.query({
53+
sql,
54+
supportBigNumbers: '',
55+
bigNumberStrings: '',
56+
});
57+
58+
assert.strictEqual(
59+
typeof results[0].total,
60+
'number',
61+
'bigNumberStrings as an empty string should be disabled',
62+
);
63+
}),
64+
]).then(async () => {
65+
await connection.end();
66+
});

‎test/esm/unit/parsers/support-big-numbers-sanitization.test.mjs‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { describe, test, assert } from 'poku';
2+
import { createConnection, describeOptions } from '../../../common.test.cjs';
3+
4+
const connection = createConnection().promise();
5+
6+
const sql = 'SELECT 9007199254740991+100 AS `total`';
7+
8+
describe('supportBigNumbers Sanitization', describeOptions);
9+
10+
Promise.all([
11+
test(async () => {
12+
const [results] = await connection.query({
13+
sql,
14+
supportBigNumbers: true,
15+
});
16+
17+
assert.strictEqual(
18+
typeof results[0].total,
19+
'string',
20+
'Valid supportBigNumbers enabled',
21+
);
22+
}),
23+
test(async () => {
24+
const [results] = await connection.query({
25+
sql,
26+
supportBigNumbers: false,
27+
});
28+
29+
assert.strictEqual(
30+
typeof results[0].total,
31+
'number',
32+
'Valid supportBigNumbers disabled',
33+
);
34+
}),
35+
36+
test(async () => {
37+
const [results] = await connection.query({
38+
sql,
39+
supportBigNumbers: 'text',
40+
});
41+
42+
assert.strictEqual(
43+
typeof results[0].total,
44+
'string',
45+
'supportBigNumbers as a random string should be enabled',
46+
);
47+
}),
48+
test(async () => {
49+
const [results] = await connection.query({
50+
sql,
51+
supportBigNumbers: '',
52+
});
53+
54+
assert.strictEqual(
55+
typeof results[0].total,
56+
'number',
57+
'supportBigNumbers as an empty string should be disabled',
58+
);
59+
}),
60+
]).then(async () => {
61+
await connection.end();
62+
});

0 commit comments

Comments
 (0)