Skip to content

Commit

Permalink
Obsolete literal AddColumn
Browse files Browse the repository at this point in the history
SqlInsert/UpdateBuilder AddColumn overloads taking a value have a SQL injection vulnerability, and have no usage.
  • Loading branch information
fredericDelaporte committed Apr 2, 2024
1 parent 1605507 commit d7ee3fa
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 18 deletions.
23 changes: 9 additions & 14 deletions src/NHibernate.Test/SqlCommandTest/SqlInsertBuilderFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using NHibernate.Engine;
using NHibernate.SqlCommand;
using NHibernate.SqlTypes;
using NHibernate.Type;
using NUnit.Framework;

namespace NHibernate.Test.SqlCommandTest
Expand All @@ -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);
Expand All @@ -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");
}

Expand All @@ -66,27 +63,25 @@ 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);

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");
}
}
}
}
6 changes: 2 additions & 4 deletions src/NHibernate.Test/SqlCommandTest/SqlUpdateBuilderFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -60,4 +58,4 @@ public void UpdateStringSqlTest()
Assert.AreEqual(expectedParameterTypes[3], actualParameterTypes[3], "fourthParam Type");
}
}
}
}
2 changes: 2 additions & 0 deletions src/NHibernate/SqlCommand/SqlInsertBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ public virtual SqlInsertBuilder AddColumn(string columnName, IType propertyType)
/// <param name="val">The value to set for the column.</param>
/// <param name="literalType">The NHibernateType to use to convert the value to a sql string.</param>
/// <returns>The SqlInsertBuilder.</returns>
// 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));
Expand Down
2 changes: 2 additions & 0 deletions src/NHibernate/SqlCommand/SqlUpdateBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ public SqlUpdateBuilder SetComment(string comment)
/// <param name="val">The value to set for the column.</param>
/// <param name="literalType">The NHibernateType to use to convert the value to a sql string.</param>
/// <returns>The SqlUpdateBuilder.</returns>
// 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));
Expand Down

0 comments on commit d7ee3fa

Please sign in to comment.