-
Notifications
You must be signed in to change notification settings - Fork 934
Handle SQL injection vulnerabilities within ObjectToSQLString #3547
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
Merged
fredericDelaporte
merged 26 commits into
nhibernate:5.4.x
from
fredericDelaporte:sql-injection
Jul 2, 2024
Merged
Changes from 20 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
d4b9daf
Add injection test cases
fredericDelaporte a3619ac
Add a discriminator injection test
fredericDelaporte b7a153a
Add a test for special characters
fredericDelaporte d2ff013
Minor code cleanup
hazzik d480863
Add test for char
fredericDelaporte 61a9549
Minor code cleanup
hazzik 897a869
Initial is reserved keyword in oracle
hazzik f6c3988
Add a charenum injection test
fredericDelaporte d7677c6
Add an Uri injection test case
fredericDelaporte 69c4c12
Add numerical types injection test cases
fredericDelaporte d85c5a6
Add a datetime test case
fredericDelaporte 4cf9fd3
Escapes string in AbstractStringType
fredericDelaporte 2d21ff3
Fix argument name
hazzik 0dcbfca
Fix a test failing due to new Unicode support
fredericDelaporte 2da9e9e
Fix the char type
fredericDelaporte 02bcc42
Fix types handled as SQL strings
fredericDelaporte edc4177
Add a minimal fix for numeric types
fredericDelaporte 77fe3e5
Minimal fix for the datetime case
fredericDelaporte aa91eb7
Disallow culture injection for numeric types
fredericDelaporte 03936a2
Disallow culture injecton in ticks dependent types
fredericDelaporte b7c0576
Generate async files
github-actions[bot] ea888be
Switch to cast instead of convert
fredericDelaporte 27bc4bf
Add injection test for other datetime types
fredericDelaporte 0c35063
Fix other datetime types
fredericDelaporte e80b766
Add a Guid injection test
fredericDelaporte a1cebb2
Fix the Guid type
fredericDelaporte File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
354 changes: 354 additions & 0 deletions
354
src/NHibernate.Test/Async/NHSpecificTest/GH3516/FixtureByCode.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,354 @@ | ||
//------------------------------------------------------------------------------ | ||
// <auto-generated> | ||
// This code was generated by AsyncGenerator. | ||
// | ||
// Changes to this file may cause incorrect behavior and will be lost if | ||
// the code is regenerated. | ||
// </auto-generated> | ||
//------------------------------------------------------------------------------ | ||
|
||
|
||
using System; | ||
using System.Collections; | ||
using System.Collections.Generic; | ||
using System.Globalization; | ||
using NHibernate.Cfg.MappingSchema; | ||
using NHibernate.Mapping.ByCode; | ||
using NHibernate.SqlTypes; | ||
using NHibernate.Type; | ||
using NUnit.Framework; | ||
|
||
namespace NHibernate.Test.NHSpecificTest.GH3516 | ||
{ | ||
using System.Threading.Tasks; | ||
[TestFixture] | ||
public class FixtureByCodeAsync : TestCaseMappingByCode | ||
{ | ||
protected override HbmMapping GetMappings() | ||
{ | ||
var mapper = new ModelMapper(); | ||
mapper.Class<Entity>(rc => | ||
{ | ||
rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb)); | ||
rc.Property(x => x.Name); | ||
rc.Property(x => x.FirstChar); | ||
rc.Property(x => x.CharacterEnum, m => m.Type<EnumCharType<CharEnum>>()); | ||
rc.Property(x => x.UriProperty); | ||
|
||
rc.Property(x => x.ByteProperty); | ||
rc.Property(x => x.DecimalProperty); | ||
rc.Property(x => x.DoubleProperty); | ||
rc.Property(x => x.FloatProperty); | ||
rc.Property(x => x.ShortProperty); | ||
rc.Property(x => x.IntProperty); | ||
rc.Property(x => x.LongProperty); | ||
|
||
if (TestDialect.SupportsSqlType(SqlTypeFactory.SByte)) | ||
rc.Property(x => x.SByteProperty); | ||
else | ||
_unsupportedNumericalProperties.Add(nameof(Entity.SByteProperty)); | ||
|
||
if (TestDialect.SupportsSqlType(SqlTypeFactory.UInt16)) | ||
rc.Property(x => x.UShortProperty); | ||
else | ||
_unsupportedNumericalProperties.Add(nameof(Entity.UShortProperty)); | ||
|
||
if (TestDialect.SupportsSqlType(SqlTypeFactory.UInt32)) | ||
rc.Property(x => x.UIntProperty); | ||
else | ||
_unsupportedNumericalProperties.Add(nameof(Entity.UIntProperty)); | ||
|
||
if (TestDialect.SupportsSqlType(SqlTypeFactory.UInt64)) | ||
rc.Property(x => x.ULongProperty); | ||
else | ||
_unsupportedNumericalProperties.Add(nameof(Entity.ULongProperty)); | ||
|
||
rc.Property(x => x.DateProperty); | ||
}); | ||
|
||
mapper.Class<BaseClass>(rc => | ||
{ | ||
rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb)); | ||
rc.Discriminator(x => x.Column("StringDiscriminator")); | ||
rc.Property(x => x.Name); | ||
rc.Abstract(true); | ||
}); | ||
mapper.Subclass<Subclass1>(rc => rc.DiscriminatorValue(Entity.NameWithSingleQuote)); | ||
mapper.Subclass<Subclass2>(rc => rc.DiscriminatorValue(Entity.NameWithEscapedSingleQuote)); | ||
|
||
mapper.Import<CharEnum>(); | ||
|
||
return mapper.CompileMappingForAllExplicitlyAddedEntities(); | ||
} | ||
|
||
private CultureInfo _backupCulture; | ||
private CultureInfo _backupUICulture; | ||
|
||
protected override void OnSetUp() | ||
{ | ||
using var session = OpenSession(); | ||
using var transaction = session.BeginTransaction(); | ||
session.Save( | ||
new Entity | ||
{ | ||
Name = Entity.NameWithSingleQuote, | ||
FirstChar = Entity.QuoteInitial, | ||
CharacterEnum = CharEnum.SingleQuote, | ||
UriProperty = Entity.UriWithSingleQuote | ||
}); | ||
session.Save( | ||
new Entity | ||
{ | ||
Name = Entity.NameWithEscapedSingleQuote, | ||
FirstChar = Entity.BackslashInitial, | ||
CharacterEnum = CharEnum.Backslash, | ||
UriProperty = Entity.UriWithEscapedSingleQuote | ||
}); | ||
|
||
transaction.Commit(); | ||
|
||
_backupCulture = CultureInfo.CurrentCulture; | ||
_backupUICulture = CultureInfo.CurrentUICulture; | ||
} | ||
|
||
protected override void OnTearDown() | ||
{ | ||
if (_backupCulture != null) | ||
{ | ||
CultureInfo.CurrentCulture = _backupCulture; | ||
CultureInfo.CurrentUICulture = _backupUICulture; | ||
} | ||
|
||
using var session = OpenSession(); | ||
using var transaction = session.BeginTransaction(); | ||
session.CreateQuery("delete from System.Object").ExecuteUpdate(); | ||
|
||
transaction.Commit(); | ||
} | ||
|
||
private static readonly string[] StringInjectionsProperties = | ||
{ | ||
nameof(Entity.NameWithSingleQuote), nameof(Entity.NameWithEscapedSingleQuote) | ||
}; | ||
|
||
[TestCaseSource(nameof(StringInjectionsProperties))] | ||
public void SqlInjectionInStringsAsync(string propertyName) | ||
{ | ||
using var session = OpenSession(); | ||
|
||
var query = session.CreateQuery($"from Entity e where e.Name = Entity.{propertyName}"); | ||
IList<Entity> list = null; | ||
Assert.That(async () => list = await (query.ListAsync<Entity>()), Throws.Nothing); | ||
Assert.That(list, Has.Count.EqualTo(1), $"Unable to find entity with name {propertyName}"); | ||
} | ||
|
||
private static readonly string[] SpecialNames = | ||
{ | ||
"\0; drop table Entity; --", | ||
"\b; drop table Entity; --", | ||
"\n; drop table Entity; --", | ||
"\r; drop table Entity; --", | ||
"\t; drop table Entity; --", | ||
"\x1A; drop table Entity; --", | ||
"\"; drop table Entity; --", | ||
"\\; drop table Entity; --" | ||
}; | ||
|
||
[TestCaseSource(nameof(SpecialNames))] | ||
public async Task StringsWithSpecialCharactersAsync(string name) | ||
{ | ||
// We may not even be able to insert the entity. | ||
var wasInserted = false; | ||
try | ||
{ | ||
using var s = OpenSession(); | ||
using var t = s.BeginTransaction(); | ||
var e = new Entity { Name = name }; | ||
await (s.SaveAsync(e)); | ||
await (t.CommitAsync()); | ||
|
||
wasInserted = true; | ||
} | ||
catch (Exception e) | ||
{ | ||
Assert.Warn($"The entity insertion failed with message {e}"); | ||
} | ||
|
||
try | ||
{ | ||
using var session = OpenSession(); | ||
Entity.ArbitraryStringValue = name; | ||
var list = await (session.CreateQuery($"from Entity e where e.Name = Entity.{nameof(Entity.ArbitraryStringValue)}").ListAsync<Entity>()); | ||
if (wasInserted && list.Count != 1) | ||
Assert.Warn($"Unable to find entity with name {nameof(Entity.ArbitraryStringValue)}"); | ||
} | ||
catch (Exception e) | ||
{ | ||
Assert.Warn($"The query has failed with message {e}"); | ||
} | ||
|
||
// Check the db is not wrecked. | ||
if (wasInserted) | ||
{ | ||
using var session = OpenSession(); | ||
var list = await (session | ||
.CreateQuery("from Entity e where e.Name = :name") | ||
.SetString("name", name) | ||
.ListAsync<Entity>()); | ||
Assert.That(list, Has.Count.EqualTo(1)); | ||
} | ||
else | ||
{ | ||
using var session = OpenSession(); | ||
var all = await (session.CreateQuery("from Entity e").ListAsync<Entity>()); | ||
Assert.That(all, Has.Count.GreaterThan(0)); | ||
} | ||
} | ||
|
||
[Test] | ||
public async Task SqlInjectionInStringDiscriminatorAsync() | ||
{ | ||
using var session = OpenSession(); | ||
|
||
await (session.SaveAsync(new Subclass1 { Name = "Subclass1" })); | ||
await (session.SaveAsync(new Subclass2 { Name = "Subclass2" })); | ||
|
||
// ObjectToSQLString is used for generating the inserts. | ||
Assert.That(session.Flush, Throws.Nothing, "Unable to flush the subclasses"); | ||
|
||
foreach (var entityName in new[] { nameof(Subclass1), nameof(Subclass2) }) | ||
{ | ||
var query = session.CreateQuery($"from {entityName}"); | ||
IList list = null; | ||
Assert.That(async () => list = await (query.ListAsync()), Throws.Nothing, $"Unable to list entities of {entityName}"); | ||
Assert.That(list, Has.Count.EqualTo(1), $"Unable to find the {entityName} entity"); | ||
} | ||
} | ||
|
||
private static readonly string[] CharInjectionsProperties = | ||
{ | ||
nameof(Entity.QuoteInitial), nameof(Entity.BackslashInitial) | ||
}; | ||
|
||
[TestCaseSource(nameof(CharInjectionsProperties))] | ||
public void SqlInjectionInCharAsync(string propertyName) | ||
{ | ||
using var session = OpenSession(); | ||
var query = session.CreateQuery($"from Entity e where e.FirstChar = Entity.{propertyName}"); | ||
IList<Entity> list = null; | ||
Assert.That(async () => list = await (query.ListAsync<Entity>()), Throws.Nothing); | ||
Assert.That(list, Is.Not.Null.And.Count.EqualTo(1), $"Unable to find entity with initial {propertyName}"); | ||
} | ||
|
||
private static readonly string[] CharEnumInjections = | ||
{ | ||
nameof(CharEnum.SingleQuote), nameof(CharEnum.Backslash) | ||
}; | ||
|
||
[TestCaseSource(nameof(CharEnumInjections))] | ||
public void SqlInjectionWithCharEnumAsync(string enumName) | ||
{ | ||
using var session = OpenSession(); | ||
|
||
var query = session.CreateQuery($"from Entity e where e.CharacterEnum = CharEnum.{enumName}"); | ||
IList<Entity> list = null; | ||
Assert.That(async () => list = await (query.ListAsync<Entity>()), Throws.Nothing); | ||
Assert.That(list, Has.Count.EqualTo(1), $"Unable to find entity with CharacterEnum {enumName}"); | ||
} | ||
|
||
private static readonly string[] UriInjections = | ||
{ | ||
nameof(Entity.UriWithSingleQuote), nameof(Entity.UriWithEscapedSingleQuote) | ||
}; | ||
|
||
[TestCaseSource(nameof(UriInjections))] | ||
public void SqlInjectionWithUriAsync(string propertyName) | ||
{ | ||
using var session = OpenSession(); | ||
|
||
var query = session.CreateQuery($"from Entity e where e.UriProperty = Entity.{propertyName}"); | ||
IList<Entity> list = null; | ||
Assert.That(async () => list = await (query.ListAsync<Entity>()), Throws.Nothing); | ||
Assert.That(list, Has.Count.EqualTo(1), $"Unable to find entity with UriProperty {propertyName}"); | ||
} | ||
|
||
private static readonly string[] NumericalTypesInjections = | ||
{ | ||
nameof(Entity.ByteProperty), | ||
nameof(Entity.DecimalProperty), | ||
nameof(Entity.DoubleProperty), | ||
nameof(Entity.FloatProperty), | ||
nameof(Entity.ShortProperty), | ||
nameof(Entity.IntProperty), | ||
nameof(Entity.LongProperty), | ||
nameof(Entity.SByteProperty), | ||
nameof(Entity.UShortProperty), | ||
nameof(Entity.UIntProperty), | ||
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"); | ||
|
||
Entity.ArbitraryStringValue = "0; drop table Entity; --"; | ||
using (var session = OpenSession()) | ||
{ | ||
IQuery query; | ||
// Defining that query is invalid and should throw. | ||
try | ||
{ | ||
query = session.CreateQuery($"from Entity e where e.{propertyName} = Entity.{nameof(Entity.ArbitraryStringValue)}"); | ||
} | ||
catch (Exception ex) | ||
{ | ||
// All good. | ||
Assert.Pass($"The wicked query creation has been rejected, as it should: {ex}"); | ||
// Needed for the compiler who does not know "Pass" always throw. | ||
return; | ||
} | ||
|
||
// The query definition has been accepted, run it. | ||
try | ||
{ | ||
await (query.ListAsync<Entity>()); | ||
} | ||
catch (Exception ex) | ||
{ | ||
// Expecting no exception at that point, but the test is to check if the injection succeeded. | ||
Assert.Warn($"The wicked query execution has failed: {ex}"); | ||
} | ||
} | ||
|
||
// Check if we can still query Entity. If it succeeds, at least it means the injection failed. | ||
using (var session = OpenSession()) | ||
{ | ||
IList<Entity> list = null; | ||
Assert.That(async () => list = await (session.CreateQuery("from Entity e").ListAsync<Entity>()), Throws.Nothing); | ||
Assert.That(list, Has.Count.GreaterThan(0)); | ||
} | ||
} | ||
|
||
[Test] | ||
public void SqlInjectionWithDatetimeAsync() | ||
{ | ||
var wickedCulture = new CultureInfo("en-US"); | ||
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"); | ||
IList<Entity> list = null; | ||
Assume.That(() => list = query.List<Entity>(), Throws.Nothing, | ||
"The first execution of the query failed, the injection has likely failed"); | ||
// Execute again to check the table is still here. | ||
Assert.That(async () => list = await (query.ListAsync<Entity>()), Throws.Nothing, | ||
"The second execution of the query failed although the first one did not: the injection has succeeded"); | ||
} | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.