Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
181 changes: 181 additions & 0 deletions src/webapp01/Pages/DevSecOps7.cshtml
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
@page
@model DevSecOps7Model
@{
ViewData["Title"] = "DevSecOps 7 - GitHub Advanced Security";
}

<div class="container">
<div class="row">
<div class="col-12">
<h1 class="display-4 text-primary">@ViewData["Title"]</h1>
<p class="lead">Explore the cutting-edge features and capabilities of GitHub Advanced Security (GHAS)</p>
<hr />
</div>
</div>

<!-- Alert for TempData messages -->
@if (TempData["RegexResult"] != null)
{
<div class="alert alert-info alert-dismissible fade show" role="alert">
@TempData["RegexResult"]
<button type="button" class="btn-close" data-bs-dismiss="alert" aria-label="Close"></button>
</div>
}

@if (TempData["RegexError"] != null)
{
<div class="alert alert-danger alert-dismissible fade show" role="alert">
@TempData["RegexError"]
<button type="button" class="btn-close" data-bs-dismiss="alert" aria-label="Close"></button>
</div>
}

<div class="row">
<!-- Latest GHAS News Section -->
<div class="col-lg-8">
<div class="card mb-4">
<div class="card-header bg-dark text-white">
<h3 class="card-title mb-0">
<i class="bi bi-shield-check"></i> Latest GitHub Advanced Security News
</h3>
</div>
<div class="card-body">
@if (Model.LatestNews.Any())
{
<div class="list-group list-group-flush">
@foreach (var newsItem in Model.LatestNews)
{
<div class="list-group-item d-flex align-items-start">
<span class="badge bg-success rounded-pill me-3 mt-1">NEW</span>
<div>
<p class="mb-1">@newsItem</p>
<small class="text-muted">Updated: @DateTime.Now.ToString("MMM dd, yyyy")</small>
</div>
</div>
}
</div>
}
else
{
<p class="text-muted">No news available at this time.</p>
}
</div>
</div>

<!-- GHAS Features Overview -->
<div class="card mb-4">
<div class="card-header bg-primary text-white">
<h3 class="card-title mb-0">Core GHAS Features</h3>
</div>
<div class="card-body">
<div class="row">
<div class="col-md-6">
<h5><i class="bi bi-search"></i> Code Scanning</h5>
<p>Automated vulnerability detection using CodeQL semantic analysis engine.</p>

<h5><i class="bi bi-key"></i> Secret Scanning</h5>
<p>Detect and prevent secrets from being committed to repositories.</p>
</div>
<div class="col-md-6">
<h5><i class="bi bi-layers"></i> Dependency Review</h5>
<p>Understand security impact of dependency changes in pull requests.</p>

<h5><i class="bi bi-graph-up"></i> Security Overview</h5>
<p>Organization-wide security posture visibility and compliance tracking.</p>
</div>
</div>
</div>
</div>
</div>

<!-- Sidebar with Demo Tools -->
<div class="col-lg-4">
<!-- Security Demo Section -->
<div class="card mb-4">
<div class="card-header bg-warning text-dark">
<h4 class="card-title mb-0">
<i class="bi bi-exclamation-triangle"></i> Security Demo
</h4>
</div>
<div class="card-body">
<p class="text-muted small">
This page contains intentionally vulnerable code for demonstration purposes.
These vulnerabilities should be detected by GHAS code scanning.
</p>

<!-- Regex Testing Form -->
<form method="post" asp-page-handler="TestRegex" class="mt-3">
<div class="mb-3">
<label for="pattern" class="form-label">Test Regex Pattern:</label>
<input type="text" class="form-control" id="pattern" name="pattern"
placeholder="Enter pattern (e.g., aaa)" value="aaa">
<div class="form-text">
⚠️ This uses a vulnerable regex pattern susceptible to ReDoS attacks.
</div>
</div>
<button type="submit" class="btn btn-warning btn-sm">
<i class="bi bi-play"></i> Test Pattern
</button>
</form>
</div>
</div>

<!-- Quick Links -->
<div class="card">
<div class="card-header bg-info text-white">
<h4 class="card-title mb-0">Quick Links</h4>
</div>
<div class="card-body">
<div class="d-grid gap-2">
<a href="https://docs.github.com/en/code-security" class="btn btn-outline-primary btn-sm" target="_blank">
<i class="bi bi-book"></i> GHAS Documentation
</a>
<a href="https://github.com/github/codeql" class="btn btn-outline-secondary btn-sm" target="_blank">
<i class="bi bi-github"></i> CodeQL Repository
</a>
<a href="https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/about-code-scanning" class="btn btn-outline-success btn-sm" target="_blank">
<i class="bi bi-shield-check"></i> Code Scanning Guide
</a>
<a href="https://docs.github.com/en/code-security/secret-scanning" class="btn btn-outline-warning btn-sm" target="_blank">
<i class="bi bi-key"></i> Secret Scanning
</a>
</div>
</div>
</div>
</div>
</div>

<!-- Footer Section -->
<div class="row mt-5">
<div class="col-12">
<div class="alert alert-light" role="alert">
<h5 class="alert-heading">
<i class="bi bi-lightbulb"></i> Pro Tip:
</h5>
<p>
Enable GitHub Advanced Security on your repositories to automatically detect the
security vulnerabilities demonstrated in this page's source code. GHAS will identify
issues like hardcoded credentials, vulnerable regex patterns, and potential log injection attacks.
</p>
<hr>
<p class="mb-0">
Learn more about implementing a comprehensive DevSecOps strategy with
<a href="https://github.com/features/security" target="_blank">GitHub Advanced Security</a>.
</p>
</div>
</div>
</div>
</div>

@section Scripts {
<script>
// Simple script to auto-dismiss alerts after 5 seconds
setTimeout(function() {
const alerts = document.querySelectorAll('.alert-dismissible');
alerts.forEach(alert => {
const bsAlert = new bootstrap.Alert(alert);
bsAlert.close();
});
}, 5000);
</script>
}
107 changes: 107 additions & 0 deletions src/webapp01/Pages/DevSecOps7.cshtml.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.RazorPages;
using System.Text.RegularExpressions;
using Microsoft.Data.SqlClient;
using Newtonsoft.Json;
using System.Text.Json;

namespace webapp01.Pages
{
public class DevSecOps7Model : PageModel
{
private readonly ILogger<DevSecOps7Model> _logger;

// Hardcoded credentials for demo purposes - INSECURE
private const string CONNECTION_STRING = "Server=localhost;Database=TestDB;User Id=admin;Password=SecretPassword123!;";

// Weak regex pattern - vulnerable to ReDoS
private static readonly Regex VulnerableRegex = new Regex(@"^(a+)+$", RegexOptions.Compiled);

public DevSecOps7Model(ILogger<DevSecOps7Model> logger)
{
_logger = logger;
}

public List<string> LatestNews { get; set; } = new();

public void OnGet()
{
// Log forging vulnerability - user input directly in logs
string userInput = Request.Query.ContainsKey("user") ? Request.Query["user"].ToString() ?? "anonymous" : "anonymous";

Check notice

Code scanning / CodeQL

Inefficient use of ContainsKey Note

Inefficient use of 'ContainsKey' and
indexer
.

Copilot Autofix

AI 2 days ago

To fix the inefficient use of ContainsKey and the indexer on Request.Query, replace the two operations with a single TryGetValue call. Specifically, in the OnGet method on line 30 in file src/webapp01/Pages/DevSecOps7.cshtml.cs, update the code so that Request.Query.TryGetValue("user", out var userVals) is used. This can then safely use the value if it exists, defaulting to "anonymous" if it doesn't or if conversion fails. The change is localized to line 30, and does not require any new imports, as all necessary functionality is provided by ASP.NET Core.

Suggested changeset 1
src/webapp01/Pages/DevSecOps7.cshtml.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/webapp01/Pages/DevSecOps7.cshtml.cs b/src/webapp01/Pages/DevSecOps7.cshtml.cs
--- a/src/webapp01/Pages/DevSecOps7.cshtml.cs
+++ b/src/webapp01/Pages/DevSecOps7.cshtml.cs
@@ -27,7 +27,7 @@
         public void OnGet()
         {
             // Log forging vulnerability - user input directly in logs
-            string userInput = Request.Query.ContainsKey("user") ? Request.Query["user"].ToString() ?? "anonymous" : "anonymous";
+            string userInput = Request.Query.TryGetValue("user", out var userVals) ? userVals.ToString() ?? "anonymous" : "anonymous";
             _logger.LogInformation($"User accessed DevSecOps7 page: {userInput}");
 
             // Simulate getting latest news about GitHub Advanced Security
EOF
@@ -27,7 +27,7 @@
public void OnGet()
{
// Log forging vulnerability - user input directly in logs
string userInput = Request.Query.ContainsKey("user") ? Request.Query["user"].ToString() ?? "anonymous" : "anonymous";
string userInput = Request.Query.TryGetValue("user", out var userVals) ? userVals.ToString() ?? "anonymous" : "anonymous";
_logger.LogInformation($"User accessed DevSecOps7 page: {userInput}");

// Simulate getting latest news about GitHub Advanced Security
Copilot is powered by AI and may make mistakes. Always verify output.
_logger.LogInformation($"User accessed DevSecOps7 page: {userInput}");

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.

Copilot Autofix

AI 2 days ago

To fix the vulnerability, user input that is being logged should be sanitized so that log forging via new lines or format-breaking characters is not possible. Since the log entry is written as plain text (not HTML), the recommended technique is to strip out all line breaks and similar control characters from the value before including it in the log. This can be done using string.Replace for \r and \n. Additionally, optionally wrapping or quoting the value helps clarify the log entries. The only necessary change is on the log call in line 31 within src/webapp01/Pages/DevSecOps7.cshtml.cs, where the userInput variable is sanitized in the log message.

If reused elsewhere, it may be beneficial to create a small helper for sanitization, but for now, only demonstrated where flagged.


Suggested changeset 1
src/webapp01/Pages/DevSecOps7.cshtml.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/webapp01/Pages/DevSecOps7.cshtml.cs b/src/webapp01/Pages/DevSecOps7.cshtml.cs
--- a/src/webapp01/Pages/DevSecOps7.cshtml.cs
+++ b/src/webapp01/Pages/DevSecOps7.cshtml.cs
@@ -28,7 +28,9 @@
         {
             // Log forging vulnerability - user input directly in logs
             string userInput = Request.Query.ContainsKey("user") ? Request.Query["user"].ToString() ?? "anonymous" : "anonymous";
-            _logger.LogInformation($"User accessed DevSecOps7 page: {userInput}");
+            // Sanitize user input before logging to prevent log forging
+            string sanitizedUserInput = userInput.Replace("\r", "").Replace("\n", "");
+            _logger.LogInformation($"User accessed DevSecOps7 page: {sanitizedUserInput}");
 
             // Simulate getting latest news about GitHub Advanced Security
             LoadLatestGHASNews();
EOF
@@ -28,7 +28,9 @@
{
// Log forging vulnerability - user input directly in logs
string userInput = Request.Query.ContainsKey("user") ? Request.Query["user"].ToString() ?? "anonymous" : "anonymous";
_logger.LogInformation($"User accessed DevSecOps7 page: {userInput}");
// Sanitize user input before logging to prevent log forging
string sanitizedUserInput = userInput.Replace("\r", "").Replace("\n", "");
_logger.LogInformation($"User accessed DevSecOps7 page: {sanitizedUserInput}");

// Simulate getting latest news about GitHub Advanced Security
LoadLatestGHASNews();
Copilot is powered by AI and may make mistakes. Always verify output.

// Simulate getting latest news about GitHub Advanced Security
LoadLatestGHASNews();

// Demonstrate potential ReDoS vulnerability
string testPattern = Request.Query.ContainsKey("pattern") ? Request.Query["pattern"].ToString() ?? "aaa" : "aaa";

Check notice

Code scanning / CodeQL

Inefficient use of ContainsKey Note

Inefficient use of 'ContainsKey' and
indexer
.

Copilot Autofix

AI 2 days ago

To fix the inefficient use of ContainsKey followed by index access, replace the pattern:

string testPattern = Request.Query.ContainsKey("pattern") ? Request.Query["pattern"].ToString() ?? "aaa" : "aaa";

with a call to TryGetValue, which combines both operations efficiently. The best way is to declare a local variable for the value retrieved (of type Microsoft.Extensions.Primitives.StringValues), then use .ToString() or fallback logic as needed. The replacement should still ensure the value is assigned as "aaa" if the query param is absent or its value is null. No new imports or methods are needed; this change is limited to the code at line 37.


Suggested changeset 1
src/webapp01/Pages/DevSecOps7.cshtml.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/webapp01/Pages/DevSecOps7.cshtml.cs b/src/webapp01/Pages/DevSecOps7.cshtml.cs
--- a/src/webapp01/Pages/DevSecOps7.cshtml.cs
+++ b/src/webapp01/Pages/DevSecOps7.cshtml.cs
@@ -34,7 +34,15 @@
             LoadLatestGHASNews();
 
             // Demonstrate potential ReDoS vulnerability
-            string testPattern = Request.Query.ContainsKey("pattern") ? Request.Query["pattern"].ToString() ?? "aaa" : "aaa";
+            string testPattern;
+            if (Request.Query.TryGetValue("pattern", out var patternVal) && patternVal.ToString() != null)
+            {
+                testPattern = patternVal.ToString();
+            }
+            else
+            {
+                testPattern = "aaa";
+            }
             try
             {
                 bool isMatch = VulnerableRegex.IsMatch(testPattern);
EOF
@@ -34,7 +34,15 @@
LoadLatestGHASNews();

// Demonstrate potential ReDoS vulnerability
string testPattern = Request.Query.ContainsKey("pattern") ? Request.Query["pattern"].ToString() ?? "aaa" : "aaa";
string testPattern;
if (Request.Query.TryGetValue("pattern", out var patternVal) && patternVal.ToString() != null)
{
testPattern = patternVal.ToString();
}
else
{
testPattern = "aaa";
}
try
{
bool isMatch = VulnerableRegex.IsMatch(testPattern);
Copilot is powered by AI and may make mistakes. Always verify output.
try
{
bool isMatch = VulnerableRegex.IsMatch(testPattern);

Check failure

Code scanning / CodeQL

Denial of Service from comparison of user input against expensive regex High

This regex operation with dangerous complexity depends on a
user-provided value
.

Copilot Autofix

AI 2 days ago

The best way to fix the problem is to ensure that any regex operation on attacker-controlled input either (a) uses a regular expression with guaranteed low complexity, or (b) sets a timeout on the regex evaluation, preventing ReDoS. Since the purpose of this demo is to show security practices, it's prudent to fix the dangerous usage by using the overload of the .NET Regex methods (e.g., IsMatch) or the Regex constructor that accepts a TimeSpan timeout, thus ensuring that even if the pattern or input is problematic, the evaluation will be aborted after a safe period (such as 1 second).

In this specific context, lines which call VulnerableRegex.IsMatch(testPattern) (line 40 and line 94) are the main concern. The existing VulnerableRegex is a static field constructed without a timeout. Instead of using it, construct a regex instance with a timeout for each request, or update the static regex to use a timeout. Since static initialization with TimeSpan is tricky (as there's no static constructor with timeout in use right now), the best fix is to replace the match check with a new Regex instance for each match, using the same pattern, but specifying a safe timeout.

You will need to add using System; if not already present (it isn't strictly needed in this context since TimeSpan.FromSeconds(...) is fully qualified or can be written with System.TimeSpan.FromSeconds). Otherwise, it's safe.

Specifically:

  • On line 40: Replace VulnerableRegex.IsMatch(testPattern) with new Regex(@"^(a+)+$", RegexOptions.Compiled, TimeSpan.FromSeconds(1)).IsMatch(testPattern);
  • On line 94: Replace VulnerableRegex.IsMatch(pattern) similarly.
  • Optionally, remove the static VulnerableRegex if it is not needed elsewhere.

Suggested changeset 1
src/webapp01/Pages/DevSecOps7.cshtml.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/webapp01/Pages/DevSecOps7.cshtml.cs b/src/webapp01/Pages/DevSecOps7.cshtml.cs
--- a/src/webapp01/Pages/DevSecOps7.cshtml.cs
+++ b/src/webapp01/Pages/DevSecOps7.cshtml.cs
@@ -15,7 +15,7 @@
         private const string CONNECTION_STRING = "Server=localhost;Database=TestDB;User Id=admin;Password=SecretPassword123!;";
         
         // Weak regex pattern - vulnerable to ReDoS
-        private static readonly Regex VulnerableRegex = new Regex(@"^(a+)+$", RegexOptions.Compiled);
+        // (Removed VulnerableRegex; use safer inline construction with timeout)
 
         public DevSecOps7Model(ILogger<DevSecOps7Model> logger)
         {
@@ -37,7 +37,7 @@
             string testPattern = Request.Query.ContainsKey("pattern") ? Request.Query["pattern"].ToString() ?? "aaa" : "aaa";
             try
             {
-                bool isMatch = VulnerableRegex.IsMatch(testPattern);
+                bool isMatch = new Regex(@"^(a+)+$", RegexOptions.Compiled, TimeSpan.FromSeconds(1)).IsMatch(testPattern);
                 _logger.LogInformation($"Regex pattern match result: {isMatch} for input: {testPattern}");
             }
             catch (Exception ex)
@@ -91,7 +91,7 @@
             try
             {
                 // Vulnerable regex that could cause ReDoS
-                bool result = VulnerableRegex.IsMatch(pattern);
+                bool result = new Regex(@"^(a+)+$", RegexOptions.Compiled, TimeSpan.FromSeconds(1)).IsMatch(pattern);
                 TempData["RegexResult"] = $"Pattern '{pattern}' match result: {result}";
             }
             catch (Exception ex)
EOF
@@ -15,7 +15,7 @@
private const string CONNECTION_STRING = "Server=localhost;Database=TestDB;User Id=admin;Password=SecretPassword123!;";

// Weak regex pattern - vulnerable to ReDoS
private static readonly Regex VulnerableRegex = new Regex(@"^(a+)+$", RegexOptions.Compiled);
// (Removed VulnerableRegex; use safer inline construction with timeout)

public DevSecOps7Model(ILogger<DevSecOps7Model> logger)
{
@@ -37,7 +37,7 @@
string testPattern = Request.Query.ContainsKey("pattern") ? Request.Query["pattern"].ToString() ?? "aaa" : "aaa";
try
{
bool isMatch = VulnerableRegex.IsMatch(testPattern);
bool isMatch = new Regex(@"^(a+)+$", RegexOptions.Compiled, TimeSpan.FromSeconds(1)).IsMatch(testPattern);
_logger.LogInformation($"Regex pattern match result: {isMatch} for input: {testPattern}");
}
catch (Exception ex)
@@ -91,7 +91,7 @@
try
{
// Vulnerable regex that could cause ReDoS
bool result = VulnerableRegex.IsMatch(pattern);
bool result = new Regex(@"^(a+)+$", RegexOptions.Compiled, TimeSpan.FromSeconds(1)).IsMatch(pattern);
TempData["RegexResult"] = $"Pattern '{pattern}' match result: {result}";
}
catch (Exception ex)
Copilot is powered by AI and may make mistakes. Always verify output.
_logger.LogInformation($"Regex pattern match result: {isMatch} for input: {testPattern}");

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.

Copilot Autofix

AI 2 days ago

To fix this vulnerability, sanitize the user input (testPattern) before logging it, to prevent malicious actors from injecting log-forging characters (such as newlines). For plain text logs (as in this code), the best mitigation is to strip out all newlines and possibly other control characters from the user input prior to logging. This can be done using String.Replace for \r, \n, or—for better security—Regex.Replace to remove all control characters.
You should make the minimal change necessary for correct behavior: replace the use of testPattern in the log message(s) (line 41 and any similar cases) with a sanitized version that removes newlines (e.g. testPattern.Replace("\n", "").Replace("\r", "") or testPatternSanitized). This can be either done inline or (preferably) by assigning to a new variable for clarity.
As you only have access to the code in src/webapp01/Pages/DevSecOps7.cshtml.cs, apply this change only to the scope of the specific log message(s) involving tainted user input.

Suggested changeset 1
src/webapp01/Pages/DevSecOps7.cshtml.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/webapp01/Pages/DevSecOps7.cshtml.cs b/src/webapp01/Pages/DevSecOps7.cshtml.cs
--- a/src/webapp01/Pages/DevSecOps7.cshtml.cs
+++ b/src/webapp01/Pages/DevSecOps7.cshtml.cs
@@ -35,10 +35,12 @@
 
             // Demonstrate potential ReDoS vulnerability
             string testPattern = Request.Query.ContainsKey("pattern") ? Request.Query["pattern"].ToString() ?? "aaa" : "aaa";
+            // Sanitize user input to prevent log forging
+            string testPatternSanitized = testPattern.Replace("\r", "").Replace("\n", "");
             try
             {
                 bool isMatch = VulnerableRegex.IsMatch(testPattern);
-                _logger.LogInformation($"Regex pattern match result: {isMatch} for input: {testPattern}");
+                _logger.LogInformation($"Regex pattern match result: {isMatch} for input: {testPatternSanitized}");
             }
             catch (Exception ex)
             {
EOF
@@ -35,10 +35,12 @@

// Demonstrate potential ReDoS vulnerability
string testPattern = Request.Query.ContainsKey("pattern") ? Request.Query["pattern"].ToString() ?? "aaa" : "aaa";
// Sanitize user input to prevent log forging
string testPatternSanitized = testPattern.Replace("\r", "").Replace("\n", "");
try
{
bool isMatch = VulnerableRegex.IsMatch(testPattern);
_logger.LogInformation($"Regex pattern match result: {isMatch} for input: {testPattern}");
_logger.LogInformation($"Regex pattern match result: {isMatch} for input: {testPatternSanitized}");
}
catch (Exception ex)
{
Copilot is powered by AI and may make mistakes. Always verify output.
}
catch (Exception ex)
{
// Log forging in exception handling
_logger.LogError($"Regex evaluation failed for pattern: {testPattern}. Error: {ex.Message}");

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.

Copilot Autofix

AI 2 days ago

To fix the problem, we should sanitize any user input before it is included in log entries. Since the logging targets text logs (not HTML), the main danger is control characters such as newlines, carriage returns, and tabs, which can be used to forge/mislead log entries. The appropriate remediation is to remove these characters from any user-supplied input before logging.

A suitable fix is to call .Replace("\r", "") and .Replace("\n", "") on the value, or use a regular expression to remove all possible problematic control characters as shown in examples (or at minimum, \r and \n). In the case of testPattern, apply this sanitization before interpolating it into the log message at line 46.

A similar change should be applied to other log entries in the file that incorporate unsanitized user input (e.g., lines 31, 41, 89, 100, and possibly 95), but the direct fix required by the error is for line 46.

Update line 46 in src/webapp01/Pages/DevSecOps7.cshtml.cs as follows:

  • Before logging, sanitize testPattern by removing newlines/carriage returns (e.g. testPatternSanitized = testPattern.Replace("\r", "").Replace("\n", ""))
  • Use the sanitized value in the log message.

If broader sanitization is desired, consider encapsulating the logic in a helper method within the file.


Suggested changeset 1
src/webapp01/Pages/DevSecOps7.cshtml.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/webapp01/Pages/DevSecOps7.cshtml.cs b/src/webapp01/Pages/DevSecOps7.cshtml.cs
--- a/src/webapp01/Pages/DevSecOps7.cshtml.cs
+++ b/src/webapp01/Pages/DevSecOps7.cshtml.cs
@@ -43,7 +43,8 @@
             catch (Exception ex)
             {
                 // Log forging in exception handling
-                _logger.LogError($"Regex evaluation failed for pattern: {testPattern}. Error: {ex.Message}");
+                var testPatternSanitized = testPattern.Replace("\r", "").Replace("\n", "");
+                _logger.LogError($"Regex evaluation failed for pattern: {testPatternSanitized}. Error: {ex.Message}");
             }
 
             // Simulate database connection with hardcoded credentials
EOF
@@ -43,7 +43,8 @@
catch (Exception ex)
{
// Log forging in exception handling
_logger.LogError($"Regex evaluation failed for pattern: {testPattern}. Error: {ex.Message}");
var testPatternSanitized = testPattern.Replace("\r", "").Replace("\n", "");
_logger.LogError($"Regex evaluation failed for pattern: {testPatternSanitized}. Error: {ex.Message}");
}

// Simulate database connection with hardcoded credentials
Copilot is powered by AI and may make mistakes. Always verify output.
}
Comment on lines +43 to +47

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.

Copilot Autofix

AI 2 days ago

To fix this error, we should update the catch (Exception ex) block on line 43 to instead catch only the specific exceptions that are reasonably expected from Regex.IsMatch. According to Microsoft's documentation, these typically are:

  • RegexMatchTimeoutException: Thrown when the match time-out interval is exceeded.
  • ArgumentNullException: If testPattern is null (but the code defaults testPattern to "aaa", so this is unlikely, but may occur depending on refactoring).
  • RegexParseException (added in .NET 7; in earlier versions, you'd get ArgumentException): When the regex pattern itself is invalid. However, here the regex pattern is constant and safe; only the input string is dynamic, so this may not apply.

Strictly, the most likely runtime exception here is RegexMatchTimeoutException. It's safe to catch that specifically. Optionally, ArgumentNullException can be caught as a fallback.

Therefore, in the try-catch block that wraps VulnerableRegex.IsMatch(testPattern), replace

catch (Exception ex)

with

catch (RegexMatchTimeoutException ex)
catch (ArgumentNullException ex)

ensuring the handler's body is the same (just logging).

To implement this, we may need to add a using System; import (but that's already included by default, so this is not required) and ensure RegexMatchTimeoutException is in scope (it is, via System.Text.RegularExpressions).

Lines to change:
Only the catch block at line 43–47 in src/webapp01/Pages/DevSecOps7.cshtml.cs needs changing.


Suggested changeset 1
src/webapp01/Pages/DevSecOps7.cshtml.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/webapp01/Pages/DevSecOps7.cshtml.cs b/src/webapp01/Pages/DevSecOps7.cshtml.cs
--- a/src/webapp01/Pages/DevSecOps7.cshtml.cs
+++ b/src/webapp01/Pages/DevSecOps7.cshtml.cs
@@ -40,11 +40,15 @@
                 bool isMatch = VulnerableRegex.IsMatch(testPattern);
                 _logger.LogInformation($"Regex pattern match result: {isMatch} for input: {testPattern}");
             }
-            catch (Exception ex)
+            catch (RegexMatchTimeoutException ex)
             {
                 // Log forging in exception handling
-                _logger.LogError($"Regex evaluation failed for pattern: {testPattern}. Error: {ex.Message}");
+                _logger.LogError($"Regex evaluation timed out for pattern: {testPattern}. Error: {ex.Message}");
             }
+            catch (ArgumentNullException ex)
+            {
+                _logger.LogError($"Regex evaluation failed due to null input for pattern: {testPattern}. Error: {ex.Message}");
+            }
 
             // Simulate database connection with hardcoded credentials
             try
EOF
@@ -40,11 +40,15 @@
bool isMatch = VulnerableRegex.IsMatch(testPattern);
_logger.LogInformation($"Regex pattern match result: {isMatch} for input: {testPattern}");
}
catch (Exception ex)
catch (RegexMatchTimeoutException ex)
{
// Log forging in exception handling
_logger.LogError($"Regex evaluation failed for pattern: {testPattern}. Error: {ex.Message}");
_logger.LogError($"Regex evaluation timed out for pattern: {testPattern}. Error: {ex.Message}");
}
catch (ArgumentNullException ex)
{
_logger.LogError($"Regex evaluation failed due to null input for pattern: {testPattern}. Error: {ex.Message}");
}

// Simulate database connection with hardcoded credentials
try
Copilot is powered by AI and may make mistakes. Always verify output.

// Simulate database connection with hardcoded credentials
try
{
using var connection = new SqlConnection(CONNECTION_STRING);

Check failure

Code scanning / CodeQL

Insecure SQL connection High

Connection string
flows to this SQL connection and does not specify Encrypt=True.

Copilot Autofix

AI 2 days ago

To fix the problem, edit the connection string specified within the CONNECTION_STRING constant (line 15) to include the parameter Encrypt=True;. This change enforces that the client-side connection to SQL Server will use encrypted transport (TLS), mitigating the identified risk. No additional code changes are required elsewhere, as the connection string is only used for the demonstration SQL connection instantiation.

Suggested changeset 1
src/webapp01/Pages/DevSecOps7.cshtml.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/webapp01/Pages/DevSecOps7.cshtml.cs b/src/webapp01/Pages/DevSecOps7.cshtml.cs
--- a/src/webapp01/Pages/DevSecOps7.cshtml.cs
+++ b/src/webapp01/Pages/DevSecOps7.cshtml.cs
@@ -12,7 +12,7 @@
         private readonly ILogger<DevSecOps7Model> _logger;
 
         // Hardcoded credentials for demo purposes - INSECURE
-        private const string CONNECTION_STRING = "Server=localhost;Database=TestDB;User Id=admin;Password=SecretPassword123!;";
+        private const string CONNECTION_STRING = "Server=localhost;Database=TestDB;User Id=admin;Password=SecretPassword123!;Encrypt=True;";
         
         // Weak regex pattern - vulnerable to ReDoS
         private static readonly Regex VulnerableRegex = new Regex(@"^(a+)+$", RegexOptions.Compiled);
EOF
@@ -12,7 +12,7 @@
private readonly ILogger<DevSecOps7Model> _logger;

// Hardcoded credentials for demo purposes - INSECURE
private const string CONNECTION_STRING = "Server=localhost;Database=TestDB;User Id=admin;Password=SecretPassword123!;";
private const string CONNECTION_STRING = "Server=localhost;Database=TestDB;User Id=admin;Password=SecretPassword123!;Encrypt=True;";

// Weak regex pattern - vulnerable to ReDoS
private static readonly Regex VulnerableRegex = new Regex(@"^(a+)+$", RegexOptions.Compiled);
Copilot is powered by AI and may make mistakes. Always verify output.
_logger.LogInformation("Attempting database connection...");
// Don't actually open connection for demo purposes
}
catch (Exception ex)
{
_logger.LogError($"Database connection failed: {ex.Message}");
}
Comment on lines +56 to +59

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.

Copilot Autofix

AI 2 days ago

To resolve the problem, we should replace catch (Exception ex) on line 56 with a catch block for the specific exception type expected from the database connection attempt, which is SqlException. The code in question specifically attempts to connect to a database using SqlConnection, and according to Microsoft documentation, failed connection attempts will throw a SqlException. Therefore, in file src/webapp01/Pages/DevSecOps7.cshtml.cs, within the method OnGet, change the catch block as follows:

  • Replace catch (Exception ex) with catch (SqlException ex).
  • No additional using directives are needed as Microsoft.Data.SqlClient is already imported on line 4.

Other aspects of the code remain unchanged. No new dependencies or libraries are required.


Suggested changeset 1
src/webapp01/Pages/DevSecOps7.cshtml.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/webapp01/Pages/DevSecOps7.cshtml.cs b/src/webapp01/Pages/DevSecOps7.cshtml.cs
--- a/src/webapp01/Pages/DevSecOps7.cshtml.cs
+++ b/src/webapp01/Pages/DevSecOps7.cshtml.cs
@@ -53,7 +53,7 @@
                 _logger.LogInformation("Attempting database connection...");
                 // Don't actually open connection for demo purposes
             }
-            catch (Exception ex)
+            catch (SqlException ex)
             {
                 _logger.LogError($"Database connection failed: {ex.Message}");
             }
EOF
@@ -53,7 +53,7 @@
_logger.LogInformation("Attempting database connection...");
// Don't actually open connection for demo purposes
}
catch (Exception ex)
catch (SqlException ex)
{
_logger.LogError($"Database connection failed: {ex.Message}");
}
Copilot is powered by AI and may make mistakes. Always verify output.
}

private void LoadLatestGHASNews()
{
LatestNews = new List<string>
{
"GitHub Advanced Security now supports enhanced code scanning with CodeQL 2.20",
"New secret scanning patterns added for over 200 service providers",
"Dependency review alerts now include detailed remediation guidance",
"Security advisories integration improved for better vulnerability management",
"Custom CodeQL queries can now be shared across organizations",
"AI-powered security suggestions available in GitHub Copilot for Security",
"New compliance frameworks supported in security overview dashboard",
"Enhanced SARIF support for third-party security tools integration"
};

// Potential JSON deserialization vulnerability
string jsonData = JsonConvert.SerializeObject(LatestNews);
var deserializedData = JsonConvert.DeserializeObject<List<string>>(jsonData);

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
deserializedData
is useless, since its value is never read.

Copilot Autofix

AI 2 days ago

To fix the problem, we should remove the assignment to the unused local variable (deserializedData) on line 78 within the LoadLatestGHASNews method in src/webapp01/Pages/DevSecOps7.cshtml.cs. Since the value of deserializedData is never used or read, and the purpose of deserializing is not critical as per the provided code context, the best approach is to remove the assignment entirely. If the deserialization method call (JsonConvert.DeserializeObject<List<string>>(jsonData)) is not required for its side-effects (which is extremely unlikely here as jsonData is derived from hardcoded data), it can be safely removed from the function. No imports or definitions need to be added or changed, as this is simply deleting an unused assignment.


Suggested changeset 1
src/webapp01/Pages/DevSecOps7.cshtml.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/webapp01/Pages/DevSecOps7.cshtml.cs b/src/webapp01/Pages/DevSecOps7.cshtml.cs
--- a/src/webapp01/Pages/DevSecOps7.cshtml.cs
+++ b/src/webapp01/Pages/DevSecOps7.cshtml.cs
@@ -75,7 +75,6 @@
 
             // Potential JSON deserialization vulnerability
             string jsonData = JsonConvert.SerializeObject(LatestNews);
-            var deserializedData = JsonConvert.DeserializeObject<List<string>>(jsonData);
             
             _logger.LogInformation($"Loaded {LatestNews.Count} news items about GitHub Advanced Security");
         }
EOF
@@ -75,7 +75,6 @@

// Potential JSON deserialization vulnerability
string jsonData = JsonConvert.SerializeObject(LatestNews);
var deserializedData = JsonConvert.DeserializeObject<List<string>>(jsonData);

_logger.LogInformation($"Loaded {LatestNews.Count} news items about GitHub Advanced Security");
}
Copilot is powered by AI and may make mistakes. Always verify output.

_logger.LogInformation($"Loaded {LatestNews.Count} news items about GitHub Advanced Security");
}

public IActionResult OnPostTestRegex(string pattern)
{
if (string.IsNullOrEmpty(pattern))
return BadRequest("Pattern cannot be empty");

// Log forging vulnerability in POST handler
_logger.LogInformation($"Testing regex pattern submitted by user: {pattern}");

try
{
// Vulnerable regex that could cause ReDoS
bool result = VulnerableRegex.IsMatch(pattern);
TempData["RegexResult"] = $"Pattern '{pattern}' match result: {result}";
}
catch (Exception ex)
{
// Logging sensitive information
_logger.LogError($"Regex test failed for pattern: {pattern}. Exception: {ex}");
TempData["RegexError"] = "Pattern evaluation failed";
}
Comment on lines +97 to +102

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.

Copilot Autofix

AI 2 days ago

To fix the problem, replace the broad catch (catch (Exception ex)) with more precise exception handling. In the context of regex matching with Regex.IsMatch, the expected exceptions are RegexMatchTimeoutException (if a match times out) and ArgumentException (if the pattern is invalid or malformed). Therefore, change the catch block to individually handle these exceptions. Make sure to log and report the error for these specific cases, while letting other exceptions propagate up (so the framework will handle them and log appropriately).
Edit only the block in the OnPostTestRegex method in src/webapp01/Pages/DevSecOps7.cshtml.cs, lines 97-102. No new imports are needed, as these exceptions are in System.


Suggested changeset 1
src/webapp01/Pages/DevSecOps7.cshtml.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/webapp01/Pages/DevSecOps7.cshtml.cs b/src/webapp01/Pages/DevSecOps7.cshtml.cs
--- a/src/webapp01/Pages/DevSecOps7.cshtml.cs
+++ b/src/webapp01/Pages/DevSecOps7.cshtml.cs
@@ -94,12 +94,16 @@
                 bool result = VulnerableRegex.IsMatch(pattern);
                 TempData["RegexResult"] = $"Pattern '{pattern}' match result: {result}";
             }
-            catch (Exception ex)
+            catch (RegexMatchTimeoutException ex)
             {
-                // Logging sensitive information
-                _logger.LogError($"Regex test failed for pattern: {pattern}. Exception: {ex}");
-                TempData["RegexError"] = "Pattern evaluation failed";
+                _logger.LogError($"Regex test timed out for pattern: {pattern}. Exception: {ex}");
+                TempData["RegexError"] = "Pattern evaluation timed out";
             }
+            catch (ArgumentException ex)
+            {
+                _logger.LogError($"Invalid regex pattern submitted: {pattern}. Exception: {ex}");
+                TempData["RegexError"] = "Invalid regex pattern";
+            }
 
             return RedirectToPage();
         }
EOF
@@ -94,12 +94,16 @@
bool result = VulnerableRegex.IsMatch(pattern);
TempData["RegexResult"] = $"Pattern '{pattern}' match result: {result}";
}
catch (Exception ex)
catch (RegexMatchTimeoutException ex)
{
// Logging sensitive information
_logger.LogError($"Regex test failed for pattern: {pattern}. Exception: {ex}");
TempData["RegexError"] = "Pattern evaluation failed";
_logger.LogError($"Regex test timed out for pattern: {pattern}. Exception: {ex}");
TempData["RegexError"] = "Pattern evaluation timed out";
}
catch (ArgumentException ex)
{
_logger.LogError($"Invalid regex pattern submitted: {pattern}. Exception: {ex}");
TempData["RegexError"] = "Invalid regex pattern";
}

return RedirectToPage();
}
Copilot is powered by AI and may make mistakes. Always verify output.

return RedirectToPage();
}
}
}
2 changes: 1 addition & 1 deletion src/webapp01/Pages/Index.cshtml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<p class="card-text">Learn about <a href="https://learn.microsoft.com/aspnet/core">building Web apps with ASP.NET Core</a>.</p>
<p class="card-text">Visit our <a asp-page="/About">About GHAS</a> page to learn about GitHub Advanced Security features.</p>
<p class="card-text">
<strong>New!</strong> Check out our <a asp-page="/DevSecOps" class="btn btn-primary btn-sm">DevSecOps Demo</a>
<strong>New!</strong> Check out our <a asp-page="/DevSecOps7" class="btn btn-primary btn-sm">DevSecOps7 Demo</a>
page to see the latest GHAS features and security demonstrations.
</p>
</div>
Expand Down
2 changes: 1 addition & 1 deletion src/webapp01/webapp01.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<PackageReference Include="Microsoft.Data.SqlClient" Version="5.0.2" />
<PackageReference Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.21.0" />
<PackageReference Include="System.Text.Json" Version="8.0.4" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
<PackageReference Include="Newtonsoft.Json" Version="12.0.2" />
</ItemGroup>

</Project>
Loading