From d7ee3fa092d4d8ead447e710fc0ae17826923d7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericdelaporte@users.noreply.github.com> Date: Tue, 2 Apr 2024 13:41:29 +0200 Subject: [PATCH] Obsolete literal AddColumn SqlInsert/UpdateBuilder AddColumn overloads taking a value have a SQL injection vulnerability, and have no usage. --- .../SqlCommandTest/SqlInsertBuilderFixture.cs | 23 ++++++++----------- .../SqlCommandTest/SqlUpdateBuilderFixture.cs | 6 ++--- src/NHibernate/SqlCommand/SqlInsertBuilder.cs | 2 ++ src/NHibernate/SqlCommand/SqlUpdateBuilder.cs | 2 ++ 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/NHibernate.Test/SqlCommandTest/SqlInsertBuilderFixture.cs b/src/NHibernate.Test/SqlCommandTest/SqlInsertBuilderFixture.cs index 795bddf586c..86eedf17b77 100644 --- a/src/NHibernate.Test/SqlCommandTest/SqlInsertBuilderFixture.cs +++ b/src/NHibernate.Test/SqlCommandTest/SqlInsertBuilderFixture.cs @@ -2,7 +2,6 @@ using NHibernate.Engine; using NHibernate.SqlCommand; using NHibernate.SqlTypes; -using NHibernate.Type; using NUnit.Framework; namespace NHibernate.Test.SqlCommandTest @@ -26,15 +25,13 @@ public void InsertSqlStringTest() insert.AddColumn("intColumn", NHibernateUtil.Int32); insert.AddColumn("longColumn", NHibernateUtil.Int64); - insert.AddColumn("literalColumn", false, (ILiteralType) NHibernateUtil.Boolean); insert.AddColumn("stringColumn", 5.ToString()); SqlCommandInfo sqlCommand = insert.ToSqlCommandInfo(); SqlType[] actualParameterTypes = sqlCommand.ParameterTypes; - string falseString = factoryImpl.Dialect.ToBooleanValueString(false); string expectedSql = - "INSERT INTO test_insert_builder (intColumn, longColumn, literalColumn, stringColumn) VALUES (?, ?, " + falseString + ", 5)"; + "INSERT INTO test_insert_builder (intColumn, longColumn, stringColumn) VALUES (?, ?, 5)"; Assert.AreEqual(expectedSql, sqlCommand.Text.ToString(), "SQL String"); Assert.AreEqual(2, actualParameterTypes.Length); @@ -48,15 +45,15 @@ public void Commented() Configuration cfg = new Configuration(); ISessionFactory factory = cfg.BuildSessionFactory(); - ISessionFactoryImplementor factoryImpl = (ISessionFactoryImplementor)factory; + ISessionFactoryImplementor factoryImpl = (ISessionFactoryImplementor) factory; SqlInsertBuilder insert = new SqlInsertBuilder(factoryImpl); insert.SetTableName("test_insert_builder"); - insert.AddColumn("stringColumn", "aSQLValue", (ILiteralType)NHibernateUtil.String); + insert.AddColumn("intColumn", NHibernateUtil.Int32); insert.SetComment("Test insert"); string expectedSql = - "/* Test insert */ INSERT INTO test_insert_builder (stringColumn) VALUES ('aSQLValue')"; + "/* Test insert */ INSERT INTO test_insert_builder (intColumn) VALUES (?)"; Assert.AreEqual(expectedSql, insert.ToSqlString().ToString(), "SQL String"); } @@ -66,12 +63,11 @@ public void MixingParametersAndValues() Configuration cfg = new Configuration(); ISessionFactory factory = cfg.BuildSessionFactory(); - ISessionFactoryImplementor factoryImpl = (ISessionFactoryImplementor)factory; + ISessionFactoryImplementor factoryImpl = (ISessionFactoryImplementor) factory; SqlInsertBuilder insert = new SqlInsertBuilder(factoryImpl); insert.SetTableName("test_insert_builder"); - insert.AddColumn("literalColumn", false, (ILiteralType)NHibernateUtil.Boolean); insert.AddColumn("intColumn", NHibernateUtil.Int32); insert.AddColumn("stringColumn", 5.ToString()); insert.AddColumn("longColumn", NHibernateUtil.Int64); @@ -79,14 +75,13 @@ public void MixingParametersAndValues() SqlCommandInfo sqlCommand = insert.ToSqlCommandInfo(); SqlType[] actualParameterTypes = sqlCommand.ParameterTypes; - string falseString = factoryImpl.Dialect.ToBooleanValueString(false); - string expectedSql = - "INSERT INTO test_insert_builder (literalColumn, intColumn, stringColumn, longColumn) VALUES (" + falseString + ", ?, 5, ?)"; + string expectedSql = + "INSERT INTO test_insert_builder (intColumn, stringColumn, longColumn) VALUES (?, 5, ?)"; Assert.AreEqual(expectedSql, sqlCommand.Text.ToString(), "SQL String"); Assert.AreEqual(2, actualParameterTypes.Length); Assert.AreEqual(SqlTypeFactory.Int32, actualParameterTypes[0], "First Parameter Type"); - Assert.AreEqual(SqlTypeFactory.Int64, actualParameterTypes[1], "Second Parameter Type"); + Assert.AreEqual(SqlTypeFactory.Int64, actualParameterTypes[1], "Second Parameter Type"); } } -} \ No newline at end of file +} diff --git a/src/NHibernate.Test/SqlCommandTest/SqlUpdateBuilderFixture.cs b/src/NHibernate.Test/SqlCommandTest/SqlUpdateBuilderFixture.cs index c0bd0ece22b..ac5359ddd10 100644 --- a/src/NHibernate.Test/SqlCommandTest/SqlUpdateBuilderFixture.cs +++ b/src/NHibernate.Test/SqlCommandTest/SqlUpdateBuilderFixture.cs @@ -28,7 +28,6 @@ public void UpdateStringSqlTest() update.AddColumns(new string[] {"intColumn"}, NHibernateUtil.Int32); update.AddColumns(new string[] {"longColumn"}, NHibernateUtil.Int64); - update.AddColumn("literalColumn", false, (ILiteralType) NHibernateUtil.Boolean); update.AddColumn("stringColumn", 5.ToString()); update.SetIdentityColumn(new string[] {"decimalColumn"}, NHibernateUtil.Decimal); @@ -38,9 +37,8 @@ public void UpdateStringSqlTest() SqlCommandInfo sqlCommand = update.ToSqlCommandInfo(); Assert.AreEqual(CommandType.Text, sqlCommand.CommandType); - string falseString = factoryImpl.Dialect.ToBooleanValueString(false); string expectedSql = - "UPDATE test_update_builder SET intColumn = ?, longColumn = ?, literalColumn = " + falseString + ", stringColumn = 5 WHERE decimalColumn = ? AND versionColumn = ? AND a=b"; + "UPDATE test_update_builder SET intColumn = ?, longColumn = ?, stringColumn = 5 WHERE decimalColumn = ? AND versionColumn = ? AND a=b"; Assert.AreEqual(expectedSql, sqlCommand.Text.ToString(), "SQL String"); SqlType[] actualParameterTypes = sqlCommand.ParameterTypes; @@ -60,4 +58,4 @@ public void UpdateStringSqlTest() Assert.AreEqual(expectedParameterTypes[3], actualParameterTypes[3], "fourthParam Type"); } } -} \ No newline at end of file +} diff --git a/src/NHibernate/SqlCommand/SqlInsertBuilder.cs b/src/NHibernate/SqlCommand/SqlInsertBuilder.cs index 0a8f88883f8..d71774848b9 100644 --- a/src/NHibernate/SqlCommand/SqlInsertBuilder.cs +++ b/src/NHibernate/SqlCommand/SqlInsertBuilder.cs @@ -66,6 +66,8 @@ public virtual SqlInsertBuilder AddColumn(string columnName, IType propertyType) /// The value to set for the column. /// The NHibernateType to use to convert the value to a sql string. /// The SqlInsertBuilder. + // Since v5.6 + [Obsolete("This method is unsafe and has no more usages. Use the overload with a property type and use a parameterized query.")] public SqlInsertBuilder AddColumn(string columnName, object val, ILiteralType literalType) { return AddColumn(columnName, literalType.ObjectToSQLString(val, Dialect)); diff --git a/src/NHibernate/SqlCommand/SqlUpdateBuilder.cs b/src/NHibernate/SqlCommand/SqlUpdateBuilder.cs index 80c154ffdf8..cb4d316961d 100644 --- a/src/NHibernate/SqlCommand/SqlUpdateBuilder.cs +++ b/src/NHibernate/SqlCommand/SqlUpdateBuilder.cs @@ -47,6 +47,8 @@ public SqlUpdateBuilder SetComment(string comment) /// The value to set for the column. /// The NHibernateType to use to convert the value to a sql string. /// The SqlUpdateBuilder. + // Since v5.6 + [Obsolete("This method is unsafe and has no more usages. Use the overload with a property type and use a parameterized query.")] public SqlUpdateBuilder AddColumn(string columnName, object val, ILiteralType literalType) { return AddColumn(columnName, literalType.ObjectToSQLString(val, Dialect));