Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mysql): json path security issues #11332

Merged
merged 1 commit into from Aug 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 11 additions & 24 deletions lib/dialects/mysql/query-generator.js
Expand Up @@ -164,7 +164,7 @@ const QueryGenerator = {
// Parse nested object
if (smth.conditions) {
const conditions = _.map(this.parseConditionObject(smth.conditions), condition =>
`${this.quoteIdentifier(_.first(condition.path))}->>'\$.${_.tail(condition.path).join('.')}' = '${condition.value}'`
`${this.jsonPathExtractionQuery(condition.path[0], _.tail(condition.path))} = '${condition.value}'`
);

return conditions.join(' and ');
Expand All @@ -176,26 +176,9 @@ const QueryGenerator = {
str = smth.path;
} else {
// Also support json dot notation
let path = smth.path;
let startWithDot = true;

// Convert .number. to [number].
path = path.replace(/\.(\d+)\./g, '[$1].');
// Convert .number$ to [number]
path = path.replace(/\.(\d+)$/, '[$1]');

path = path.split('.');

let columnName = path.shift();
const match = columnName.match(/\[\d+\]$/);
// If columnName ends with [\d+]
if (match !== null) {
path.unshift(columnName.substr(match.index));
columnName = columnName.substr(0, match.index);
startWithDot = false;
}

str = `${this.quoteIdentifier(columnName)}->>'\$${startWithDot ? '.' : ''}${path.join('.')}'`;
const paths = _.toPath(smth.path);
const column = paths.shift();
str = this.jsonPathExtractionQuery(column, paths);
}

if (smth.value) {
Expand Down Expand Up @@ -474,10 +457,14 @@ const QueryGenerator = {
*
* https://bugs.mysql.com/bug.php?id=81896
*/
const paths = _.toPath(path).map(subPath => Utils.addTicks(subPath, '"'));
const pathStr = `${['$'].concat(paths).join('.')}`;
const paths = _.toPath(path).map(subPath => {
return /\D/.test(subPath)
? Utils.addTicks(subPath, '"')
: subPath;
});
const pathStr = this.escape(`${['$'].concat(paths).join('.').replace(/\.(\d+)(?:(?=\.)|$)/g, (__, digit) => `[${digit}]`)}`);
const quotedColumn = this.isIdentifierQuoted(column) ? column : this.quoteIdentifier(column);
return `(${quotedColumn}->>'${pathStr}')`;
return `(${quotedColumn}->>${pathStr})`;
},

/**
Expand Down
Empty file added test.sqlite
Empty file.
8 changes: 8 additions & 0 deletions test/integration/model/json.test.js
Expand Up @@ -684,6 +684,14 @@ describe(Support.getTestDialectTeaser('Model'), () => {
});
});

it('should properly escape path keys with sequelize.json', function() {
return this.Model.findAll({
raw: true,
attributes: ['id'],
where: this.sequelize.json("data.id')) AS DECIMAL) = 1 DELETE YOLO INJECTIONS; -- ", '1')
});
});

it('should properly escape the single quotes in array', function() {
return this.Model.create({
data: {
Expand Down
10 changes: 5 additions & 5 deletions test/unit/sql/json.test.js
Expand Up @@ -81,39 +81,39 @@ if (current.dialect.supports.JSON) {
expectsql(sql.whereItemQuery(undefined, Sequelize.json({ id: 1 })), {
postgres: '("id"#>>\'{}\') = \'1\'',
sqlite: "json_extract(`id`, '$') = '1'",
mysql: "`id`->>'$.' = '1'"
mysql: "(`id`->>'$') = '1'"
});
});

test('nested condition object', () => {
expectsql(sql.whereItemQuery(undefined, Sequelize.json({ profile: { id: 1 } })), {
postgres: '("profile"#>>\'{id}\') = \'1\'',
sqlite: "json_extract(`profile`, '$.id') = '1'",
mysql: "`profile`->>'$.id' = '1'"
mysql: "(`profile`->>'$.\\\"id\\\"') = '1'"
});
});

test('multiple condition object', () => {
expectsql(sql.whereItemQuery(undefined, Sequelize.json({ property: { value: 1 }, another: { value: 'string' } })), {
postgres: '("property"#>>\'{value}\') = \'1\' AND ("another"#>>\'{value}\') = \'string\'',
sqlite: "json_extract(`property`, '$.value') = '1' AND json_extract(`another`, '$.value') = 'string'",
mysql: "`property`->>'$.value' = '1' and `another`->>'$.value' = 'string'"
mysql: "(`property`->>'$.\\\"value\\\"') = '1' and (`another`->>'$.\\\"value\\\"') = 'string'"
});
});

test('dot notation', () => {
expectsql(sql.whereItemQuery(Sequelize.json('profile.id'), '1'), {
postgres: '("profile"#>>\'{id}\') = \'1\'',
sqlite: "json_extract(`profile`, '$.id') = '1'",
mysql: "`profile`->>'$.id' = '1'"
mysql: "(`profile`->>'$.\\\"id\\\"') = '1'"
});
});

test('column named "json"', () => {
expectsql(sql.whereItemQuery(Sequelize.json('json'), '{}'), {
postgres: '("json"#>>\'{}\') = \'{}\'',
sqlite: "json_extract(`json`, '$') = '{}'",
mysql: "`json`->>'$.' = '{}'"
mysql: "(`json`->>'$') = '{}'"
});
});
});
Expand Down
32 changes: 16 additions & 16 deletions test/unit/sql/where.test.js
Expand Up @@ -778,15 +778,15 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
expectsql(sql.whereItemQuery(undefined, this.sequelize.json('profile.id', this.sequelize.cast('12346-78912', 'text'))), {
postgres: "(\"profile\"#>>'{id}') = CAST('12346-78912' AS TEXT)",
sqlite: "json_extract(`profile`, '$.id') = CAST('12346-78912' AS TEXT)",
mysql: "`profile`->>'$.id' = CAST('12346-78912' AS CHAR)"
mysql: "(`profile`->>'$.\\\"id\\\"') = CAST('12346-78912' AS CHAR)"
});
});

test('sequelize.json({profile: {id: "12346-78912", name: "test"}})', function() {
expectsql(sql.whereItemQuery(undefined, this.sequelize.json({profile: {id: '12346-78912', name: 'test'}})), {
postgres: "(\"profile\"#>>'{id}') = '12346-78912' AND (\"profile\"#>>'{name}') = 'test'",
sqlite: "json_extract(`profile`, '$.id') = '12346-78912' AND json_extract(`profile`, '$.name') = 'test'",
mysql: "`profile`->>'$.id' = '12346-78912' and `profile`->>'$.name' = 'test'"
mysql: "(`profile`->>'$.\\\"id\\\"') = '12346-78912' and (`profile`->>'$.\\\"name\\\"') = 'test'"
});
});

Expand All @@ -800,7 +800,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
},
prefix: 'User'
}, {
mysql: "(`User`.`data`->>'$.\"nested\".\"attribute\"') = 'value'",
mysql: "(`User`.`data`->>'$.\\\"nested\\\".\\\"attribute\\\"') = 'value'",
postgres: "(\"User\".\"data\"#>>'{nested,attribute}') = 'value'",
sqlite: "json_extract(`User`.`data`, '$.nested.attribute') = 'value'"
});
Expand All @@ -814,7 +814,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
type: new DataTypes.JSONB()
}
}, {
mysql: "CAST((`data`->>'$.\"nested\"') AS DECIMAL) IN (1, 2)",
mysql: "CAST((`data`->>'$.\\\"nested\\\"') AS DECIMAL) IN (1, 2)",
postgres: "CAST((\"data\"#>>'{nested}') AS DOUBLE PRECISION) IN (1, 2)",
sqlite: "CAST(json_extract(`data`, '$.nested') AS DOUBLE PRECISION) IN (1, 2)"
});
Expand All @@ -828,7 +828,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
type: new DataTypes.JSONB()
}
}, {
mysql: "CAST((`data`->>'$.\"nested\"') AS DECIMAL) BETWEEN 1 AND 2",
mysql: "CAST((`data`->>'$.\\\"nested\\\"') AS DECIMAL) BETWEEN 1 AND 2",
postgres: "CAST((\"data\"#>>'{nested}') AS DOUBLE PRECISION) BETWEEN 1 AND 2",
sqlite: "CAST(json_extract(`data`, '$.nested') AS DOUBLE PRECISION) BETWEEN 1 AND 2"
});
Expand All @@ -846,7 +846,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
},
prefix: current.literal(sql.quoteTable.call(current.dialect.QueryGenerator, {tableName: 'User'}))
}, {
mysql: "((`User`.`data`->>'$.\"nested\".\"attribute\"') = 'value' AND (`User`.`data`->>'$.\"nested\".\"prop\"') != 'None')",
mysql: "((`User`.`data`->>'$.\\\"nested\\\".\\\"attribute\\\"') = 'value' AND (`User`.`data`->>'$.\\\"nested\\\".\\\"prop\\\"') != 'None')",
postgres: "((\"User\".\"data\"#>>'{nested,attribute}') = 'value' AND (\"User\".\"data\"#>>'{nested,prop}') != 'None')",
sqlite: "(json_extract(`User`.`data`, '$.nested.attribute') = 'value' AND json_extract(`User`.`data`, '$.nested.prop') != 'None')"
});
Expand All @@ -864,7 +864,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
},
prefix: 'User'
}, {
mysql: "((`User`.`data`->>'$.\"name\".\"last\"') = 'Simpson' AND (`User`.`data`->>'$.\"employment\"') != 'None')",
mysql: "((`User`.`data`->>'$.\\\"name\\\".\\\"last\\\"') = 'Simpson' AND (`User`.`data`->>'$.\\\"employment\\\"') != 'None')",
postgres: "((\"User\".\"data\"#>>'{name,last}') = 'Simpson' AND (\"User\".\"data\"#>>'{employment}') != 'None')",
sqlite: "(json_extract(`User`.`data`, '$.name.last') = 'Simpson' AND json_extract(`User`.`data`, '$.employment') != 'None')"
});
Expand All @@ -877,7 +877,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
type: new DataTypes.JSONB()
}
}, {
mysql: "(CAST((`data`->>'$.\"price\"') AS DECIMAL) = 5 AND (`data`->>'$.\"name\"') = 'Product')",
mysql: "(CAST((`data`->>'$.\\\"price\\\"') AS DECIMAL) = 5 AND (`data`->>'$.\\\"name\\\"') = 'Product')",
postgres: "(CAST((\"data\"#>>'{price}') AS DOUBLE PRECISION) = 5 AND (\"data\"#>>'{name}') = 'Product')",
sqlite: "(CAST(json_extract(`data`, '$.price') AS DOUBLE PRECISION) = 5 AND json_extract(`data`, '$.name') = 'Product')"
});
Expand All @@ -891,7 +891,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
}
}
}, {
mysql: "(`data`->>'$.\"nested\".\"attribute\"') = 'value'",
mysql: "(`data`->>'$.\\\"nested\\\".\\\"attribute\\\"') = 'value'",
postgres: "(\"data\"#>>'{nested,attribute}') = 'value'",
sqlite: "json_extract(`data`, '$.nested.attribute') = 'value'"
});
Expand All @@ -905,7 +905,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
}
}
}, {
mysql: "CAST((`data`->>'$.\"nested\".\"attribute\"') AS DECIMAL) = 4",
mysql: "CAST((`data`->>'$.\\\"nested\\\".\\\"attribute\\\"') AS DECIMAL) = 4",
postgres: "CAST((\"data\"#>>'{nested,attribute}') AS DOUBLE PRECISION) = 4",
sqlite: "CAST(json_extract(`data`, '$.nested.attribute') AS DOUBLE PRECISION) = 4"
});
Expand All @@ -921,7 +921,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
}
}
}, {
mysql: "CAST((`data`->>'$.\"nested\".\"attribute\"') AS DECIMAL) IN (3, 7)",
mysql: "CAST((`data`->>'$.\\\"nested\\\".\\\"attribute\\\"') AS DECIMAL) IN (3, 7)",
postgres: "CAST((\"data\"#>>'{nested,attribute}') AS DOUBLE PRECISION) IN (3, 7)",
sqlite: "CAST(json_extract(`data`, '$.nested.attribute') AS DOUBLE PRECISION) IN (3, 7)"
});
Expand All @@ -937,7 +937,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
type: new DataTypes.JSONB()
}
}, {
mysql: "CAST((`data`->>'$.\"nested\".\"attribute\"') AS DECIMAL) > 2",
mysql: "CAST((`data`->>'$.\\\"nested\\\".\\\"attribute\\\"') AS DECIMAL) > 2",
postgres: "CAST((\"data\"#>>'{nested,attribute}') AS DOUBLE PRECISION) > 2",
sqlite: "CAST(json_extract(`data`, '$.nested.attribute') AS DOUBLE PRECISION) > 2"
});
Expand All @@ -953,7 +953,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
type: new DataTypes.JSONB()
}
}, {
mysql: "CAST((`data`->>'$.\"nested\".\"attribute\"') AS DECIMAL) > 2",
mysql: "CAST((`data`->>'$.\\\"nested\\\".\\\"attribute\\\"') AS DECIMAL) > 2",
postgres: "CAST((\"data\"#>>'{nested,attribute}') AS INTEGER) > 2",
sqlite: "CAST(json_extract(`data`, '$.nested.attribute') AS INTEGER) > 2"
});
Expand All @@ -970,7 +970,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
type: new DataTypes.JSONB()
}
}, {
mysql: "CAST((`data`->>'$.\"nested\".\"attribute\"') AS DATETIME) > "+sql.escape(dt),
mysql: "CAST((`data`->>'$.\\\"nested\\\".\\\"attribute\\\"') AS DATETIME) > "+sql.escape(dt),
postgres: "CAST((\"data\"#>>'{nested,attribute}') AS TIMESTAMPTZ) > "+sql.escape(dt),
sqlite: "json_extract(`data`, '$.nested.attribute') > " + sql.escape(dt.toISOString())
});
Expand All @@ -984,7 +984,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
type: new DataTypes.JSONB()
}
}, {
mysql: "(`data`->>'$.\"nested\".\"attribute\"') = 'true'",
mysql: "(`data`->>'$.\\\"nested\\\".\\\"attribute\\\"') = 'true'",
postgres: "CAST((\"data\"#>>'{nested,attribute}') AS BOOLEAN) = true",
sqlite: "CAST(json_extract(`data`, '$.nested.attribute') AS BOOLEAN) = 1"
});
Expand All @@ -1000,7 +1000,7 @@ suite(Support.getTestDialectTeaser('SQL'), () => {
}
}
}, {
mysql: "(`meta_data`->>'$.\"nested\".\"attribute\"') = 'value'",
mysql: "(`meta_data`->>'$.\\\"nested\\\".\\\"attribute\\\"') = 'value'",
postgres: "(\"meta_data\"#>>'{nested,attribute}') = 'value'",
sqlite: "json_extract(`meta_data`, '$.nested.attribute') = 'value'"
});
Expand Down