feat: throw an error if attribute includes parentheses (fixes CVE-202…

…3-22578) (#15710)


⚠️ You may be impacted by this change if you use deprecated features. See PR for details

src/dialects/abstract/query-generator.js

@@ -1521,11 +1521,24 @@ class QueryGenerator {
         if (attr[0] instanceof Utils.SequelizeMethod) {
           attr[0] = this.handleSequelizeMethod(attr[0]);
           addTable = false;
-        } else if (!attr[0].includes('(') && !attr[0].includes(')')) {
+        } else if (this.options.attributeBehavior === 'escape' || !attr[0].includes('(') && !attr[0].includes(')')) {
           attr[0] = this.quoteIdentifier(attr[0]);
-        } else {
-          deprecations.noRawAttributes();
+        } else if (this.options.attributeBehavior !== 'unsafe-legacy') {
+          throw new Error(`Attributes cannot include parentheses in Sequelize 6:
+In order to fix the vulnerability CVE-2023-22578, we had to remove support for treating attributes as raw SQL if they included parentheses.
+Sequelize 7 escapes all attributes, even if they include parentheses.
+For Sequelize 6, because we're introducing this change in a minor release, we've opted for throwing an error instead of silently escaping the attribute as a way to warn you about this change.
+
+Here is what you can do to fix this error:
+- Wrap the attribute in a literal() call. This will make Sequelize treat it as raw SQL.
+- Set the "attributeBehavior" sequelize option to "escape" to make Sequelize escape the attribute, like in Sequelize v7. We highly recommend this option.
+- Set the "attributeBehavior" sequelize option to "unsafe-legacy" to make Sequelize escape the attribute, like in Sequelize v5.
+
+We sincerely apologize for the inconvenience this may cause you. You can find more information on the following threads:
+https://github.com/sequelize/sequelize/security/advisories/GHSA-f598-mfpv-gmfx
+https://github.com/sequelize/sequelize/discussions/15694`);
         }
+
         let alias = attr[1];
 
         if (this.options.minifyAliases) {

src/sequelize.d.ts

@@ -398,6 +398,13 @@ export interface Options extends Logging {
    * If defined the connection will use the provided schema instead of the default ("public").
    */
   schema?: string;
+
+  /**
+   * Sequelize had to introduce a breaking change to fix vulnerability CVE-2023-22578.
+   * This option allows you to revert to the old behavior (unsafe-legacy), or to opt in to the new behavior (escape).
+   * The default behavior throws an error to warn you about the change (throw).
+   */
+  attributeBehavior?: 'escape' | 'throw' | 'unsafe-legacy';
 }
 
 export interface QueryOptionsTransactionRequired { }

src/sequelize.js

@@ -292,6 +292,7 @@ class Sequelize {
       benchmark: false,
       minifyAliases: false,
       logQueryParameters: false,
+      attributeBehavior: 'throw',
       ...options
     };
 

src/utils/deprecations.ts

@@ -2,7 +2,6 @@ import { deprecate } from 'util';
 
 const noop = () => { /* noop */ };
 
-export const noRawAttributes = deprecate(noop, 'Use sequelize.fn / sequelize.literal to construct attributes', 'SEQUELIZE0001');
 export const noTrueLogging = deprecate(noop, 'The logging-option should be either a function or false. Default: console.log', 'SEQUELIZE0002');
 export const noStringOperators = deprecate(noop, 'String based operators are deprecated. Please use Symbol based operators for better security, read more at https://sequelize.org/master/manual/querying.html#operators', 'SEQUELIZE0003');
 export const noBoolOperatorAliases = deprecate(noop, 'A boolean value was passed to options.operatorsAliases. This is a no-op with v5 and should be removed.', 'SEQUELIZE0004');

test/integration/sequelize.test.js

@@ -724,7 +724,7 @@ describe(Support.getTestDialectTeaser('Sequelize'), () => {
             const TransactionTest = this.sequelizeWithTransaction.define('TransactionTest', { name: DataTypes.STRING }, { timestamps: false });
 
             const count = async transaction => {
-              const sql = this.sequelizeWithTransaction.getQueryInterface().queryGenerator.selectQuery('TransactionTests', { attributes: [['count(*)', 'cnt']] });
+              const sql = this.sequelizeWithTransaction.getQueryInterface().queryGenerator.selectQuery('TransactionTests', { attributes: [[Sequelize.literal('count(*)'), 'cnt']] });
 
               const result = await this.sequelizeWithTransaction.query(sql, { plain: true, transaction });
 
@@ -747,7 +747,7 @@ describe(Support.getTestDialectTeaser('Sequelize'), () => {
             const aliasesMapping = new Map([['_0', 'cnt']]);
 
             const count = async transaction => {
-              const sql = this.sequelizeWithTransaction.getQueryInterface().queryGenerator.selectQuery('TransactionTests', { attributes: [['count(*)', 'cnt']] });
+              const sql = this.sequelizeWithTransaction.getQueryInterface().queryGenerator.selectQuery('TransactionTests', { attributes: [[Sequelize.literal('count(*)'), 'cnt']] });
 
               const result = await this.sequelizeWithTransaction.query(sql, { plain: true, transaction, aliasesMapping  });
 

test/unit/dialects/db2/query-generator.test.js

@@ -197,7 +197,7 @@ if (dialect === 'db2') {
         }, {
           arguments: ['foo', { attributes: [['count(*)', 'count']] }],
           expectation: 'SELECT count(*) AS "count" FROM "foo";',
-          context: QueryGenerator
+          context: { options: {  attributeBehavior: 'unsafe-legacy' } }
         }, {
           arguments: ['myTable', { order: ['id'] }],
           expectation: 'SELECT * FROM "myTable" ORDER BY "id";',

test/unit/dialects/mariadb/query-generator.test.js

@@ -278,7 +278,7 @@ if (dialect === 'mariadb') {
         }, {
           arguments: ['foo', { attributes: [['count(*)', 'count']] }],
           expectation: 'SELECT count(*) AS `count` FROM `foo`;',
-          context: QueryGenerator
+          context: { options: {  attributeBehavior: 'unsafe-legacy' } }
         }, {
           arguments: ['myTable', { order: ['id'] }],
           expectation: 'SELECT * FROM `myTable` ORDER BY `id`;',

test/unit/dialects/mysql/query-generator.test.js

@@ -235,7 +235,7 @@ if (dialect === 'mysql') {
         }, {
           arguments: ['foo', { attributes: [['count(*)', 'count']] }],
           expectation: 'SELECT count(*) AS `count` FROM `foo`;',
-          context: QueryGenerator
+          context: { options: { attributeBehavior: 'unsafe-legacy' } }
         }, {
           arguments: ['myTable', { order: ['id'] }],
           expectation: 'SELECT * FROM `myTable` ORDER BY `id`;',

test/unit/dialects/postgres/query-generator.test.js

@@ -12,7 +12,7 @@ const chai = require('chai'),
   _ = require('lodash');
 
 const customSequelize = Support.createSequelizeInstance({
-  schema: 'custom',
+  schema: 'custom'
 });
 
 if (dialect.startsWith('postgres')) {
@@ -331,7 +331,8 @@ if (dialect.startsWith('postgres')) {
           expectation: 'SELECT * FROM "myTable" WHERE "myTable"."id" = 2;'
         }, {
           arguments: ['foo', { attributes: [['count(*)', 'count']] }],
-          expectation: 'SELECT count(*) AS "count" FROM "foo";'
+          expectation: 'SELECT count(*) AS "count" FROM "foo";',
+          context: { options: { attributeBehavior: 'unsafe-legacy' } }
         }, {
           arguments: ['myTable', { order: ['id'] }],
           expectation: 'SELECT * FROM "myTable" ORDER BY "id";',
@@ -526,7 +527,7 @@ if (dialect.startsWith('postgres')) {
         }, {
           arguments: ['foo', { attributes: [['count(*)', 'count']] }],
           expectation: 'SELECT count(*) AS count FROM foo;',
-          context: { options: { quoteIdentifiers: false } }
+          context: { options: { quoteIdentifiers: false, attributeBehavior: 'unsafe-legacy' } }
         }, {
           arguments: ['myTable', { order: ['id DESC'] }],
           expectation: 'SELECT * FROM myTable ORDER BY id DESC;',
@@ -1325,10 +1326,10 @@ if (dialect.startsWith('postgres')) {
     });
 
     describe('With custom schema in Sequelize options', () => {
-      beforeEach(function () {
+      beforeEach(function() {
         this.queryGenerator = new QueryGenerator({
           sequelize: customSequelize,
-          _dialect: customSequelize.dialect,
+          _dialect: customSequelize.dialect
         });
       });
 
@@ -1337,21 +1338,21 @@ if (dialect.startsWith('postgres')) {
           {
             title: 'showTablesQuery defaults to the schema set in Sequelize options',
             arguments: [],
-            expectation: `SELECT table_name FROM information_schema.tables WHERE table_schema = 'custom' AND table_type LIKE '%TABLE' AND table_name != 'spatial_ref_sys';`,
-          },
+            expectation: 'SELECT table_name FROM information_schema.tables WHERE table_schema = \'custom\' AND table_type LIKE \'%TABLE\' AND table_name != \'spatial_ref_sys\';'
+          }
         ],
         describeTableQuery: [
           {
             title: 'describeTableQuery defaults to the schema set in Sequelize options',
             arguments: ['myTable', null],
-            expectation: `SELECT pk.constraint_type as "Constraint",c.column_name as "Field", c.column_default as "Default",c.is_nullable as "Null", (CASE WHEN c.udt_name = 'hstore' THEN c.udt_name ELSE c.data_type END) || (CASE WHEN c.character_maximum_length IS NOT NULL THEN '(' || c.character_maximum_length || ')' ELSE '' END) as "Type", (SELECT array_agg(e.enumlabel) FROM pg_catalog.pg_type t JOIN pg_catalog.pg_enum e ON t.oid=e.enumtypid WHERE t.typname=c.udt_name) AS "special", (SELECT pgd.description FROM pg_catalog.pg_statio_all_tables AS st INNER JOIN pg_catalog.pg_description pgd on (pgd.objoid=st.relid) WHERE c.ordinal_position=pgd.objsubid AND c.table_name=st.relname) AS "Comment" FROM information_schema.columns c LEFT JOIN (SELECT tc.table_schema, tc.table_name, cu.column_name, tc.constraint_type FROM information_schema.TABLE_CONSTRAINTS tc JOIN information_schema.KEY_COLUMN_USAGE  cu ON tc.table_schema=cu.table_schema and tc.table_name=cu.table_name and tc.constraint_name=cu.constraint_name and tc.constraint_type='PRIMARY KEY') pk ON pk.table_schema=c.table_schema AND pk.table_name=c.table_name AND pk.column_name=c.column_name WHERE c.table_name = 'myTable' AND c.table_schema = 'custom'`,
-          },
-        ],
+            expectation: 'SELECT pk.constraint_type as "Constraint",c.column_name as "Field", c.column_default as "Default",c.is_nullable as "Null", (CASE WHEN c.udt_name = \'hstore\' THEN c.udt_name ELSE c.data_type END) || (CASE WHEN c.character_maximum_length IS NOT NULL THEN \'(\' || c.character_maximum_length || \')\' ELSE \'\' END) as "Type", (SELECT array_agg(e.enumlabel) FROM pg_catalog.pg_type t JOIN pg_catalog.pg_enum e ON t.oid=e.enumtypid WHERE t.typname=c.udt_name) AS "special", (SELECT pgd.description FROM pg_catalog.pg_statio_all_tables AS st INNER JOIN pg_catalog.pg_description pgd on (pgd.objoid=st.relid) WHERE c.ordinal_position=pgd.objsubid AND c.table_name=st.relname) AS "Comment" FROM information_schema.columns c LEFT JOIN (SELECT tc.table_schema, tc.table_name, cu.column_name, tc.constraint_type FROM information_schema.TABLE_CONSTRAINTS tc JOIN information_schema.KEY_COLUMN_USAGE  cu ON tc.table_schema=cu.table_schema and tc.table_name=cu.table_name and tc.constraint_name=cu.constraint_name and tc.constraint_type=\'PRIMARY KEY\') pk ON pk.table_schema=c.table_schema AND pk.table_name=c.table_name AND pk.column_name=c.column_name WHERE c.table_name = \'myTable\' AND c.table_schema = \'custom\''
+          }
+        ]
       };
 
       _.each(customSchemaSuites, (customSchemaTests, customSchemaSuiteTitle) => {
         for (const customSchemaTest of customSchemaTests) {
-          it(customSchemaTest.title, function () {
+          it(customSchemaTest.title, function() {
             const convertedText = customSchemaTest.arguments ? this.queryGenerator[customSchemaSuiteTitle](...customSchemaTest.arguments) : this.queryGenerator[customSchemaSuiteTitle]();
             expect(convertedText).to.equal(customSchemaTest.expectation);
           });

test/unit/dialects/snowflake/query-generator.test.js

@@ -345,7 +345,7 @@ if (dialect === 'snowflake') {
           arguments: ['myTable'],
           expectation: 'DROP TABLE IF EXISTS "myTable";'
         },
-        
+
         // Variants when quoteIdentifiers is false
         {
           arguments: ['myTable'],
@@ -357,12 +357,12 @@ if (dialect === 'snowflake') {
       tableExistsQuery: [
         {
           arguments: ['myTable'],
-          expectation: 'SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE = \'BASE TABLE\' AND TABLE_SCHEMA = CURRENT_SCHEMA() AND TABLE_NAME = \'myTable\';',
+          expectation: 'SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE = \'BASE TABLE\' AND TABLE_SCHEMA = CURRENT_SCHEMA() AND TABLE_NAME = \'myTable\';'
         },
         {
           arguments: [{ tableName: 'myTable', schema: 'mySchema' }],
-          expectation: 'SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE = \'BASE TABLE\' AND TABLE_SCHEMA = \'mySchema\' AND TABLE_NAME = \'myTable\';',
-        },
+          expectation: 'SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE = \'BASE TABLE\' AND TABLE_SCHEMA = \'mySchema\' AND TABLE_NAME = \'myTable\';'
+        }
       ],
 
       selectQuery: [
@@ -393,7 +393,7 @@ if (dialect === 'snowflake') {
         }, {
           arguments: ['foo', { attributes: [['count(*)', 'count']] }],
           expectation: 'SELECT count(*) AS "count" FROM "foo";',
-          context: QueryGenerator
+          context: { options: {  attributeBehavior: 'unsafe-legacy' } }
         }, {
           arguments: ['myTable', { order: ['id'] }],
           expectation: 'SELECT * FROM "myTable" ORDER BY "id";',
@@ -669,7 +669,7 @@ if (dialect === 'snowflake') {
         }, {
           arguments: ['foo', { attributes: [['count(*)', 'count']] }],
           expectation: 'SELECT count(*) AS count FROM foo;',
-          context: { options: { quoteIdentifiers: false } }
+          context: { options: { quoteIdentifiers: false, attributeBehavior: 'unsafe-legacy' } }
         }, {
           arguments: ['myTable', { order: ['id'] }],
           expectation: 'SELECT * FROM myTable ORDER BY id;',

test/unit/dialects/sqlite/query-generator.test.js

@@ -187,7 +187,7 @@ if (dialect === 'sqlite') {
         }, {
           arguments: ['foo', { attributes: [['count(*)', 'count']] }],
           expectation: 'SELECT count(*) AS `count` FROM `foo`;',
-          context: QueryGenerator
+          context: { options: {  attributeBehavior: 'unsafe-legacy' } }
         }, {
           arguments: ['myTable', { order: ['id'] }],
           expectation: 'SELECT * FROM `myTable` ORDER BY `id`;',

test/unit/sql/select.test.js

@@ -1083,6 +1083,39 @@ describe(Support.getTestDialectTeaser('SQL'), () => {
       });
     });
 
-  });
+    it('throws an error if encountering parentheses in an attribute', () => {
+      expect(() => sql.selectQuery(Project.tableName, {
+        model: Project,
+        attributes: [['count(*)', 'count']]
+      }, Project)).to.throw('In order to fix the vulnerability CVE-2023-22578, we had to remove support for treating attributes as raw SQL if they included parentheses.');
+    });
+
+    it('escapes attributes with parentheses if attributeBehavior is escape', () => {
+      const escapeSequelize = Support.createSequelizeInstance({
+        attributeBehavior: 'escape'
+      });
 
+      expectsql(escapeSequelize.queryInterface.queryGenerator.selectQuery(Project.tableName, {
+        model: Project,
+        attributes: [['count(*)', 'count']]
+      }, Project), {
+        default: 'SELECT [count(*)] AS [count] FROM [Projects] AS [Project];',
+        oracle: 'SELECT "count(*)" AS "count" FROM "Projects" "Project";'
+      });
+    });
+
+    it('inlines attributes with parentheses if attributeBehavior is unsafe-legacy', () => {
+      const escapeSequelize = Support.createSequelizeInstance({
+        attributeBehavior: 'unsafe-legacy'
+      });
+
+      expectsql(escapeSequelize.queryInterface.queryGenerator.selectQuery(Project.tableName, {
+        model: Project,
+        attributes: [['count(*)', 'count']]
+      }, Project), {
+        default: 'SELECT count(*) AS [count] FROM [Projects] AS [Project];',
+        oracle: 'SELECT count(*) AS "count" FROM "Projects" "Project";'
+      });
+    });
+  });
 });