Skip to content

Commit 43955af

Browse files
Dave-O-NHSDave-O-NHS
authored
APPT-2062 Fixing CSV function injection (#1635)
# Description This PR addresses security vulnerability CK2026-X3R0M-4637-01 (CSV/Formula Injection). It ensures that all exported CSV data is properly neutralized and escaped before being transmitted to the user, preventing malicious formula execution in spreadsheet applications like Microsoft Excel. # Checklist: - [ ] My work is behind a feature toggle (if appropriate) - [ ] If my work is behind a feature toggle, I've added a full suite of tests for both the ON and OFF state - [ ] The ticket number is in the Pull Request title, with format "APPT-XXX: My Title Here" - [ ] I have ran npm tsc / lint (in the future these will be ran automatically) - [ ] My code generates no new .NET warnings (in the future these will be treated as errors) - [ ] If I've added a new Function, it is disabled in all but one of the terraform groups (e.g. http_functions) - [ ] If I've added a new Function, it has both unit and integration tests. Any request body validators have unit tests also - [ ] If I've made UI changes, I've added appropriate Playwright and Jest tests - [ ] If I've added/updated an end-point, I've added the appropriate annotations and tested the Swagger documentation reflects the change
1 parent 08c7481 commit 43955af

4 files changed

Lines changed: 83 additions & 21 deletions

File tree

‎src/api/Nhs.Appointments.Core/Reports/Helpers/CsvFormatter.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
namespace Nhs.Appointments.Core.Reports.Helpers;
2+
3+
public static class CsvFormatter
4+
{
5+
private static readonly char[] FormulaTriggers = { '=', '+', '-', '@' };
6+
7+
/// <summary>
8+
/// Neutralizes CSV Injection (Formula Injection) and handles standard CSV escaping.
9+
/// </summary>
10+
public static string FormatValue(string value)
11+
{
12+
if (string.IsNullOrEmpty(value)) return "\"\"";
13+
14+
if (value.Trim().IndexOfAny(FormulaTriggers) == 0)
15+
{
16+
value = "'" + value;
17+
}
18+
19+
return "\"" + value.Replace("\"", "\"\"") + "\"";
20+
}
21+
}

‎src/api/Nhs.Appointments.Core/Reports/MasterSiteList/MasterSiteListReportCsvWriter.cs‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using Nhs.Appointments.Core.Reports.Helpers;
12
using Nhs.Appointments.Core.Sites;
23

34
namespace Nhs.Appointments.Core.Reports.MasterSiteList;
@@ -17,8 +18,6 @@ public class MasterSiteListReportCsvWriter(TimeProvider timeProvider) : IMasterS
1718
private string BuildFileName() =>
1819
$"MasterSiteListReport_{timeProvider.GetUtcNow():yyyyMMddhhmmss}.csv";
1920

20-
private static string CsvStringValue(string value) => "\"" + value + "\"";
21-
2221
private async Task CompileCsv(TextWriter csvWriter, IEnumerable<Site> sites)
2322
{
2423
await csvWriter.WriteLineAsync(string.Join(',', MasterSiteListReportMap.Headers()));
@@ -27,18 +26,18 @@ private async Task CompileCsv(TextWriter csvWriter, IEnumerable<Site> sites)
2726
{
2827
try
2928
{
30-
await csvWriter.WriteLineAsync(string.Join(',',
31-
CsvStringValue(MasterSiteListReportMap.SiteName(site)),
32-
CsvStringValue(MasterSiteListReportMap.OdsCode(site)),
33-
CsvStringValue(MasterSiteListReportMap.SiteType(site)),
34-
CsvStringValue(MasterSiteListReportMap.Region(site)),
35-
CsvStringValue(MasterSiteListReportMap.ICB(site)),
36-
CsvStringValue(MasterSiteListReportMap.Guid(site)),
29+
await csvWriter.WriteLineAsync(string.Join(',',
30+
CsvFormatter.FormatValue(MasterSiteListReportMap.SiteName(site)),
31+
CsvFormatter.FormatValue(MasterSiteListReportMap.OdsCode(site)),
32+
CsvFormatter.FormatValue(MasterSiteListReportMap.SiteType(site)),
33+
CsvFormatter.FormatValue(MasterSiteListReportMap.Region(site)),
34+
CsvFormatter.FormatValue(MasterSiteListReportMap.ICB(site)),
35+
CsvFormatter.FormatValue(MasterSiteListReportMap.Guid(site)),
3736
MasterSiteListReportMap.IsDeleted(site),
38-
CsvStringValue(MasterSiteListReportMap.Status(site)),
37+
CsvFormatter.FormatValue(MasterSiteListReportMap.Status(site)),
3938
MasterSiteListReportMap.Longitude(site),
4039
MasterSiteListReportMap.Latitude(site),
41-
CsvStringValue(MasterSiteListReportMap.Address(site))
40+
CsvFormatter.FormatValue(MasterSiteListReportMap.Address(site))
4241
));
4342

4443
}

‎src/api/Nhs.Appointments.Core/Reports/SiteSummary/SiteReportCsvWriter.cs‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using Nhs.Appointments.Core.Reports.Helpers;
12
using Nhs.Appointments.Core.Sites;
23

34
namespace Nhs.Appointments.Core.Reports.SiteSummary;
@@ -21,8 +22,6 @@ private string BuildFileName(DateOnly startDate,
2122
DateOnly endDate) =>
2223
$"SiteReport_{startDate:yyyy-MM-dd}_{endDate:yyyy-MM-dd}_{timeProvider.GetUtcNow():yyyyMMddhhmmss}.csv";
2324

24-
private static string CsvStringValue(string value) => "\"" + value + "\"";
25-
2625
private async Task CompileCsv(TextWriter csvWriter, List<SiteReport> siteReports)
2726
{
2827
var distinctServices = siteReports.SelectMany(x => x.Bookings.Keys)
@@ -33,18 +32,22 @@ private async Task CompileCsv(TextWriter csvWriter, List<SiteReport> siteReports
3332

3433
foreach (var siteReport in siteReports)
3534
{
36-
await csvWriter.WriteLineAsync(string.Join(',', CsvStringValue(SiteReportMap.SiteName(siteReport)),
37-
CsvStringValue(SiteReportMap.Status(siteReport)),
38-
CsvStringValue(SiteReportMap.SiteType(siteReport)),
39-
CsvStringValue(SiteReportMap.ICB(siteReport)), CsvStringValue(SiteReportMap.ICBName(siteReport)),
40-
CsvStringValue(SiteReportMap.Region(siteReport)), CsvStringValue(SiteReportMap.RegionName(siteReport)),
41-
CsvStringValue(SiteReportMap.OdsCode(siteReport)), SiteReportMap.Longitude(siteReport),
35+
await csvWriter.WriteLineAsync(string.Join(',',
36+
CsvFormatter.FormatValue(SiteReportMap.SiteName(siteReport)),
37+
CsvFormatter.FormatValue(SiteReportMap.Status(siteReport)),
38+
CsvFormatter.FormatValue(SiteReportMap.SiteType(siteReport)),
39+
CsvFormatter.FormatValue(SiteReportMap.ICB(siteReport)),
40+
CsvFormatter.FormatValue(SiteReportMap.ICBName(siteReport)),
41+
CsvFormatter.FormatValue(SiteReportMap.Region(siteReport)),
42+
CsvFormatter.FormatValue(SiteReportMap.RegionName(siteReport)),
43+
CsvFormatter.FormatValue(SiteReportMap.OdsCode(siteReport)),
44+
SiteReportMap.Longitude(siteReport),
4245
SiteReportMap.Latitude(siteReport),
4346
string.Join(',', distinctServices.Select(service => SiteReportMap.BookingsCount(siteReport, service))),
4447
SiteReportMap.TotalBookings(siteReport).ToString(), SiteReportMap.Cancelled(siteReport).ToString(),
4548
SiteReportMap.MaximumCapacity(siteReport).ToString(),
46-
string.Join(',',
47-
distinctServices.Select(service => SiteReportMap.CapacityCount(siteReport, service)))));
49+
string.Join(',',distinctServices.Select(service => SiteReportMap.CapacityCount(siteReport, service)))
50+
));
4851
}
4952
}
5053
}

‎tests/Nhs.Appointments.Core.UnitTests/Reports/Helpers/CsvFormatterTests.cs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
using Nhs.Appointments.Core.Reports.Helpers;
2+
3+
namespace Nhs.Appointments.Core.UnitTests.Reports.Helpers;
4+
5+
public class CsvFormatterTests
6+
{
7+
[Theory]
8+
// Standard cases
9+
[InlineData("Robin Lane", "\"Robin Lane\"")]
10+
[InlineData("", "\"\"")]
11+
[InlineData(null, "\"\"")]
12+
13+
// Security: Formula Injection neutralization
14+
[InlineData("=SUM(1,1)", "\"'=SUM(1,1)\"")]
15+
[InlineData("+44123", "\"'+44123\"")]
16+
[InlineData("-12.5", "\"'-12.5\"")]
17+
[InlineData("@username", "\"'@username\"")]
18+
19+
// These test that even if there are spaces/tabs before the trigger, we still neutralize it.
20+
[InlineData(" =SUM(1,1)", "\"' =SUM(1,1)\"")] // Leading space
21+
[InlineData(" @admin", "\"' @admin\"")] // Multiple spaces
22+
[InlineData("\t-5", "\"'\t-5\"")] // Leading tab
23+
[InlineData("\r+10", "\"'\r+10\"")] // Leading carriage return
24+
25+
// Integrity: Double quote escaping
26+
[InlineData("Health \"Express\"", "\"Health \"\"Express\"\"\"")]
27+
[InlineData("\"Quoted\"", "\"\"\"Quoted\"\"\"")]
28+
29+
// Combined: Formula + Quotes
30+
[InlineData("=A1&\"leak\"", "\"'=A1&\"\"leak\"\"\"")]
31+
public void FormatValue_ShouldNeutralizeAndEscapeCorrectly(string input, string expected)
32+
{
33+
// Act
34+
var result = CsvFormatter.FormatValue(input);
35+
36+
// Assert
37+
result.Should().Be(expected);
38+
}
39+
}

0 commit comments

Comments
 (0)