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.

Sign up for GitHub

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

Handle SQL injection vulnerabilities within ObjectToSQLString #3547

Merged
merged 26 commits into from
Jul 2, 2024
+1,105 −78
Merged

Handle SQL injection vulnerabilities within ObjectToSQLString #3547

Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d4b9daf
Add injection test cases
fredericDelaporte May 14, 2024
a3619ac
Add a discriminator injection test
fredericDelaporte Jun 9, 2024
b7a153a
Add a test for special characters
fredericDelaporte May 19, 2024
d2ff013
Minor code cleanup
hazzik Jun 3, 2024
d480863
Add test for char
fredericDelaporte May 26, 2024
61a9549
Minor code cleanup
hazzik Jun 3, 2024
897a869
Initial is reserved keyword in oracle
hazzik Jun 3, 2024
f6c3988
Add a charenum injection test
fredericDelaporte Jun 9, 2024
d7677c6
Add an Uri injection test case
fredericDelaporte Jun 9, 2024
69c4c12
Add numerical types injection test cases
fredericDelaporte Jun 9, 2024
d85c5a6
Add a datetime test case
fredericDelaporte Jun 10, 2024
4cf9fd3
Escapes string in AbstractStringType
fredericDelaporte May 12, 2024
2d21ff3
Fix argument name
hazzik Jun 3, 2024
0dcbfca
Fix a test failing due to new Unicode support
fredericDelaporte May 13, 2024
2da9e9e
Fix the char type
fredericDelaporte May 26, 2024
02bcc42
Fix types handled as SQL strings
fredericDelaporte Jun 9, 2024
edc4177
Add a minimal fix for numeric types
fredericDelaporte Jun 9, 2024
77fe3e5
Minimal fix for the datetime case
fredericDelaporte Jun 10, 2024
aa91eb7
Disallow culture injection for numeric types
fredericDelaporte Jun 11, 2024
03936a2
Disallow culture injecton in ticks dependent types
fredericDelaporte Jun 11, 2024
b7c0576
Generate async files
github-actions[bot] Jun 11, 2024
ea888be
Switch to cast instead of convert
fredericDelaporte Jun 12, 2024
27bc4bf
Add injection test for other datetime types
fredericDelaporte Jun 12, 2024
0c35063
Fix other datetime types
fredericDelaporte Jun 12, 2024
e80b766
Add a Guid injection test
fredericDelaporte Jun 12, 2024
a1cebb2
Fix the Guid type
fredericDelaporte Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add injection test for other datetime types
@fredericDelaporte
fredericDelaporte committed Jun 12, 2024
commit 27bc4bfcbcdf1afa5cb1b2794c741d4bdaa34f8b
47 changes: 35 additions & 12 deletions src/NHibernate.Test/Async/NHSpecificTest/GH3516/FixtureByCode.cs
View file
Original file line number Diff line number Diff line change
@@ -17,13 +17,17 @@
using NHibernate.SqlTypes;
using NHibernate.Type;
using NUnit.Framework;
using NUnit.Framework.Internal;

namespace NHibernate.Test.NHSpecificTest.GH3516
{
using System.Threading.Tasks;
[TestFixture]
public class FixtureByCodeAsync : TestCaseMappingByCode
{

private readonly HashSet<string> _unsupportedProperties = new();

protected override HbmMapping GetMappings()
{
var mapper = new ModelMapper();
@@ -46,24 +50,30 @@ protected override HbmMapping GetMappings()
if (TestDialect.SupportsSqlType(SqlTypeFactory.SByte))
rc.Property(x => x.SByteProperty);
else
_unsupportedNumericalProperties.Add(nameof(Entity.SByteProperty));
_unsupportedProperties.Add(nameof(Entity.SByteProperty));

if (TestDialect.SupportsSqlType(SqlTypeFactory.UInt16))
rc.Property(x => x.UShortProperty);
else
_unsupportedNumericalProperties.Add(nameof(Entity.UShortProperty));
_unsupportedProperties.Add(nameof(Entity.UShortProperty));

if (TestDialect.SupportsSqlType(SqlTypeFactory.UInt32))
rc.Property(x => x.UIntProperty);
else
_unsupportedNumericalProperties.Add(nameof(Entity.UIntProperty));
_unsupportedProperties.Add(nameof(Entity.UIntProperty));

if (TestDialect.SupportsSqlType(SqlTypeFactory.UInt64))
rc.Property(x => x.ULongProperty);
else
_unsupportedNumericalProperties.Add(nameof(Entity.ULongProperty));
_unsupportedProperties.Add(nameof(Entity.ULongProperty));

rc.Property(x => x.DateProperty);
rc.Property(x => x.DateTimeProperty);
rc.Property(x => x.DateProperty, m => m.Type(NHibernateUtil.Date));
if (TestDialect.SupportsSqlType(SqlTypeFactory.DateTimeOffSet))
rc.Property(x => x.DateTimeOffsetProperty);
else
_unsupportedProperties.Add(nameof(Entity.DateTimeOffsetProperty));
rc.Property(x => x.TimeProperty, m => m.Type(NHibernateUtil.Time));
});

mapper.Class<BaseClass>(rc =>
@@ -287,12 +297,10 @@ public void SqlInjectionWithUriAsync(string propertyName)
nameof(Entity.ULongProperty)
};

private readonly HashSet<string> _unsupportedNumericalProperties = new();

[TestCaseSource(nameof(NumericalTypesInjections))]
public async Task SqlInjectionInNumericalTypeAsync(string propertyName)
{
Assume.That(_unsupportedNumericalProperties, Does.Not.Contains((object)propertyName), $"The {propertyName} property is unsupported by the dialect");
Assume.That(_unsupportedProperties, Does.Not.Contains((object)propertyName), $"The {propertyName} property is unsupported by the dialect");

Entity.ArbitraryStringValue = "0; drop table Entity; --";
using (var session = OpenSession())
@@ -332,17 +340,32 @@ public async Task SqlInjectionInNumericalTypeAsync(string propertyName)
}
}

[Test]
public void SqlInjectionWithDatetimeAsync()
private static readonly string[] DateTypesInjections =
{
nameof(Entity.DateTimeProperty),
nameof(Entity.DateProperty),
nameof(Entity.DateTimeOffsetProperty),
nameof(Entity.TimeProperty)
};

[TestCaseSource(nameof(DateTypesInjections))]
public void SqlInjectionWithDatetimeAsync(string propertyName)
{
Assume.That(_unsupportedProperties, Does.Not.Contains((object) propertyName), $"The {propertyName} property is unsupported by the dialect");

var wickedCulture = new CultureInfo("en-US");
wickedCulture.DateTimeFormat.ShortDatePattern = "yyyy-MM-ddTHH:mm:ss\\'\"; drop table Entity; --\"";
if (propertyName == nameof(Entity.TimeProperty))
wickedCulture.DateTimeFormat.ShortTimePattern = "HH:mm:ss\\'\"; drop table Entity; --\"";
else
wickedCulture.DateTimeFormat.ShortDatePattern = "yyyy-MM-ddTHH:mm:ss\\'\"; drop table Entity; --\"";
CultureInfo.CurrentCulture = wickedCulture;
CultureInfo.CurrentUICulture = wickedCulture;

using var session = OpenSession();

var query = session.CreateQuery($"from Entity e where e.DateProperty = Entity.StaticDateProperty");
var staticPropertyName = propertyName == nameof(Entity.DateTimeOffsetProperty) ?
nameof(Entity.StaticDateTimeOffsetProperty) : nameof(Entity.StaticDateProperty);
var query = session.CreateQuery($"from Entity e where e.{propertyName} = Entity.{staticPropertyName}");
IList<Entity> list = null;
Assume.That(() => list = query.List<Entity>(), Throws.Nothing,
"The first execution of the query failed, the injection has likely failed");
4 changes: 4 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH3516/Entity.cs
View file
Original file line number Diff line number Diff line change
@@ -22,7 +22,10 @@ public class Entity
public virtual uint UIntProperty { get; set; }
public virtual ulong ULongProperty { get; set; }

public virtual DateTime DateTimeProperty { get; set; } = StaticDateProperty;
public virtual DateTime DateProperty { get; set; } = StaticDateProperty;
public virtual DateTimeOffset DateTimeOffsetProperty { get; set; } = StaticDateProperty;
public virtual DateTime TimeProperty { get; set; } = StaticDateProperty;

public const string NameWithSingleQuote = "'; drop table Entity; --";
public const string NameWithEscapedSingleQuote = @"\'; drop table Entity; --";
@@ -37,6 +40,7 @@ public class Entity
public static readonly Uri UriWithEscapedSingleQuote = new(@"https://somewhere/?sql=\'; drop table Entity; --");

public static readonly DateTime StaticDateProperty = DateTime.Today;
public static readonly DateTimeOffset StaticDateTimeOffsetProperty = DateTimeOffset.Now;
}

public enum CharEnum
47 changes: 35 additions & 12 deletions src/NHibernate.Test/NHSpecificTest/GH3516/FixtureByCode.cs
View file
Original file line number Diff line number Diff line change
@@ -7,12 +7,16 @@
using NHibernate.SqlTypes;
using NHibernate.Type;
using NUnit.Framework;
using NUnit.Framework.Internal;

namespace NHibernate.Test.NHSpecificTest.GH3516
{
[TestFixture]
public class FixtureByCode : TestCaseMappingByCode
{

private readonly HashSet<string> _unsupportedProperties = new();

protected override HbmMapping GetMappings()
{
var mapper = new ModelMapper();
@@ -35,24 +39,30 @@ protected override HbmMapping GetMappings()
if (TestDialect.SupportsSqlType(SqlTypeFactory.SByte))
rc.Property(x => x.SByteProperty);
else
_unsupportedNumericalProperties.Add(nameof(Entity.SByteProperty));
_unsupportedProperties.Add(nameof(Entity.SByteProperty));

if (TestDialect.SupportsSqlType(SqlTypeFactory.UInt16))
rc.Property(x => x.UShortProperty);
else
_unsupportedNumericalProperties.Add(nameof(Entity.UShortProperty));
_unsupportedProperties.Add(nameof(Entity.UShortProperty));

if (TestDialect.SupportsSqlType(SqlTypeFactory.UInt32))
rc.Property(x => x.UIntProperty);
else
_unsupportedNumericalProperties.Add(nameof(Entity.UIntProperty));
_unsupportedProperties.Add(nameof(Entity.UIntProperty));

if (TestDialect.SupportsSqlType(SqlTypeFactory.UInt64))
rc.Property(x => x.ULongProperty);
else
_unsupportedNumericalProperties.Add(nameof(Entity.ULongProperty));
_unsupportedProperties.Add(nameof(Entity.ULongProperty));

rc.Property(x => x.DateProperty);
rc.Property(x => x.DateTimeProperty);
rc.Property(x => x.DateProperty, m => m.Type(NHibernateUtil.Date));
if (TestDialect.SupportsSqlType(SqlTypeFactory.DateTimeOffSet))
rc.Property(x => x.DateTimeOffsetProperty);
else
_unsupportedProperties.Add(nameof(Entity.DateTimeOffsetProperty));
rc.Property(x => x.TimeProperty, m => m.Type(NHibernateUtil.Time));
});

mapper.Class<BaseClass>(rc =>
@@ -276,12 +286,10 @@ public void SqlInjectionWithUri(string propertyName)
nameof(Entity.ULongProperty)
};

private readonly HashSet<string> _unsupportedNumericalProperties = new();

[TestCaseSource(nameof(NumericalTypesInjections))]
public void SqlInjectionInNumericalType(string propertyName)
{
Assume.That(_unsupportedNumericalProperties, Does.Not.Contains((object)propertyName), $"The {propertyName} property is unsupported by the dialect");
Assume.That(_unsupportedProperties, Does.Not.Contains((object)propertyName), $"The {propertyName} property is unsupported by the dialect");

Entity.ArbitraryStringValue = "0; drop table Entity; --";
using (var session = OpenSession())
@@ -321,17 +329,32 @@ public void SqlInjectionInNumericalType(string propertyName)
}
}

[Test]
public void SqlInjectionWithDatetime()
private static readonly string[] DateTypesInjections =
{
nameof(Entity.DateTimeProperty),
nameof(Entity.DateProperty),
nameof(Entity.DateTimeOffsetProperty),
nameof(Entity.TimeProperty)
};

[TestCaseSource(nameof(DateTypesInjections))]
public void SqlInjectionWithDatetime(string propertyName)
{
Assume.That(_unsupportedProperties, Does.Not.Contains((object) propertyName), $"The {propertyName} property is unsupported by the dialect");

var wickedCulture = new CultureInfo("en-US");
wickedCulture.DateTimeFormat.ShortDatePattern = "yyyy-MM-ddTHH:mm:ss\\'\"; drop table Entity; --\"";
if (propertyName == nameof(Entity.TimeProperty))
wickedCulture.DateTimeFormat.ShortTimePattern = "HH:mm:ss\\'\"; drop table Entity; --\"";
else
wickedCulture.DateTimeFormat.ShortDatePattern = "yyyy-MM-ddTHH:mm:ss\\'\"; drop table Entity; --\"";
CultureInfo.CurrentCulture = wickedCulture;
CultureInfo.CurrentUICulture = wickedCulture;

using var session = OpenSession();

var query = session.CreateQuery($"from Entity e where e.DateProperty = Entity.StaticDateProperty");
var staticPropertyName = propertyName == nameof(Entity.DateTimeOffsetProperty) ?
nameof(Entity.StaticDateTimeOffsetProperty) : nameof(Entity.StaticDateProperty);
var query = session.CreateQuery($"from Entity e where e.{propertyName} = Entity.{staticPropertyName}");
IList<Entity> list = null;
Assume.That(() => list = query.List<Entity>(), Throws.Nothing,
"The first execution of the query failed, the injection has likely failed");