Skip to content

Commit 5fbbaba

Browse files
fix(security): prevent log forging and add workflow permissions (#51)
- Sanitize user input before logging to prevent log forging attacks (3 high-severity fixes) - Add explicit least-privilege permissions to CI/CD workflow jobs (2 medium-severity fixes) - Closes CodeQL security alerts
1 parent fa3a945 commit 5fbbaba

3 files changed

Lines changed: 34 additions & 11 deletions

File tree

.config/dotnet-tools.json

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,6 @@
88
"dotnet-csharpier"
99
],
1010
"rollForward": false
11-
},
12-
"dotnet-ef": {
13-
"version": "9.0.2",
14-
"commands": [
15-
"dotnet-ef"
16-
],
17-
"rollForward": false
1811
}
1912
}
2013
}

.github/workflows/ci-cd.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ concurrency:
2525
jobs:
2626
build-and-test:
2727
runs-on: ubuntu-latest
28+
permissions:
29+
contents: read
2830

2931
steps:
3032
- uses: actions/checkout@v4
@@ -52,6 +54,9 @@ jobs:
5254
needs: build-and-test
5355
if: github.event_name == 'release'
5456
runs-on: ubuntu-latest
57+
permissions:
58+
contents: read
59+
packages: write
5560

5661
steps:
5762
- uses: actions/checkout@v4

JsonApiToolkit/Extensions/Querying/Filtering/NestedPropertyNavigator.cs

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,37 @@
11
using System.Linq.Expressions;
22
using System.Reflection;
3+
using System.Text.RegularExpressions;
34
using JsonApiToolkit.Models.Querying.Filtering;
45
using Microsoft.Extensions.Logging;
56

67
namespace JsonApiToolkit.Extensions.Querying;
78

8-
internal static class NestedPropertyNavigator
9+
internal static partial class NestedPropertyNavigator
910
{
11+
private const int MaxLogValueLength = 100;
12+
13+
/// <summary>
14+
/// Sanitizes user input for safe logging by removing control characters
15+
/// and truncating long values to prevent log forging attacks.
16+
/// </summary>
17+
private static string SanitizeForLog(string? value)
18+
{
19+
if (string.IsNullOrEmpty(value))
20+
return "(empty)";
21+
22+
// Remove control characters (newlines, tabs, etc.) that could forge log entries
23+
string sanitized = ControlCharRegex().Replace(value, " ");
24+
25+
// Truncate long values
26+
if (sanitized.Length > MaxLogValueLength)
27+
return string.Concat(sanitized.AsSpan(0, MaxLogValueLength), "...(truncated)");
28+
29+
return sanitized;
30+
}
31+
32+
[GeneratedRegex(@"[\x00-\x1F\x7F]")]
33+
private static partial Regex ControlCharRegex();
34+
1035
internal static Expression? BuildSafeNestedFilterExpression(
1136
ParameterExpression parameter,
1237
FilterParameter filter,
@@ -268,7 +293,7 @@ internal static class NestedPropertyNavigator
268293
{
269294
logger?.LogWarning(
270295
"Failed to convert '{Value}' to {ElementType} for collection filter",
271-
filter.Value,
296+
SanitizeForLog(filter.Value),
272297
elementType.Name
273298
);
274299
return null;
@@ -295,7 +320,7 @@ internal static class NestedPropertyNavigator
295320
{
296321
logger?.LogWarning(
297322
"Failed to convert '{Value}' to {ElementType} for collection filter",
298-
filter.Value,
323+
SanitizeForLog(filter.Value),
299324
elementType.Name
300325
);
301326
return null;
@@ -412,7 +437,7 @@ internal static class NestedPropertyNavigator
412437
{
413438
logger?.LogWarning(
414439
"Failed to convert '{Value}' to {PropertyType}",
415-
filter.Value,
440+
SanitizeForLog(filter.Value),
416441
targetType.Name
417442
);
418443
return null;

0 commit comments

Comments
 (0)