Skip to content

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
merged 26 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
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 a discriminator injection test
  • Loading branch information
fredericDelaporte committed Jun 9, 2024
commit a3619ac1bd15a1e3672f8283de0c17494831e937
32 changes: 32 additions & 0 deletions src/NHibernate.Test/Async/NHSpecificTest/GH3516/FixtureByCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//------------------------------------------------------------------------------


using System.Collections;
using System.Collections.Generic;
using NHibernate.Cfg.MappingSchema;
using NHibernate.Mapping.ByCode;
Expand All @@ -27,6 +28,17 @@ protected override HbmMapping GetMappings()
rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb));
rc.Property(x => x.Name);
});

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));

return mapper.CompileMappingForAllExplicitlyAddedEntities();
}

Expand Down Expand Up @@ -68,5 +80,25 @@ public void SqlInjectionInStringsAsync(string propertyName)
Assert.That(async () => list = await (query.ListAsync<Entity>()), Throws.Nothing);
Assert.That(list, Has.Count.EqualTo(1), $"Unable to find entity with name {propertyName}");
}

[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");
}
}
}
}
34 changes: 33 additions & 1 deletion src/NHibernate.Test/NHSpecificTest/GH3516/FixtureByCode.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System.Collections;
using System.Collections.Generic;
using NHibernate.Cfg.MappingSchema;
using NHibernate.Mapping.ByCode;
using NUnit.Framework;
Expand All @@ -16,6 +17,17 @@ protected override HbmMapping GetMappings()
rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb));
rc.Property(x => x.Name);
});

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));

return mapper.CompileMappingForAllExplicitlyAddedEntities();
}

Expand Down Expand Up @@ -57,5 +69,25 @@ public void SqlInjectionInStrings(string propertyName)
Assert.That(() => list = query.List<Entity>(), Throws.Nothing);
Assert.That(list, Has.Count.EqualTo(1), $"Unable to find entity with name {propertyName}");
}

[Test]
public void SqlInjectionInStringDiscriminator()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking the trouble with string discriminator. Other discriminator types do not have the trouble, because the convert the mapping value to their type, and would fail there in case of an injection attempt.

{
using var session = OpenSession();

session.Save(new Subclass1 { Name = "Subclass1" });
session.Save(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(() => list = query.List(), Throws.Nothing, $"Unable to list entities of {entityName}");
Assert.That(list, Has.Count.EqualTo(1), $"Unable to find the {entityName} entity");
}
}
}
}
18 changes: 18 additions & 0 deletions src/NHibernate.Test/NHSpecificTest/GH3516/Hierarchy.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
using System;

namespace NHibernate.Test.NHSpecificTest.GH3516
{
public class BaseClass
{
public virtual Guid Id { get; set; }
public virtual string Name { get; set; }
}

public class Subclass1 : BaseClass
{
}

public class Subclass2 : BaseClass
{
}
}