-
Notifications
You must be signed in to change notification settings - Fork 2
Add DevSecOps7 demo page with intentionally vulnerable code for GHAS detection #113
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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> | ||
| } |
| 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 noticeCode scanning / CodeQL Inefficient use of ContainsKey Note
Inefficient use of 'ContainsKey' and
indexer Error loading related location Loading
Copilot AutofixAI about 2 months ago To fix the inefficient use of
Suggested changeset
1
src/webapp01/Pages/DevSecOps7.cshtml.cs
Copilot is powered by AI and may make mistakes. Always verify output.
Refresh and try again.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _logger.LogInformation($"User accessed DevSecOps7 page: {userInput}"); | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check failureCode scanning / CodeQL Log entries created from user input High
This log entry depends on a
user-provided value Error loading related location Loading
Copilot AutofixAI about 2 months 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 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
Copilot is powered by AI and may make mistakes. Always verify output.
Refresh and try again.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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 noticeCode scanning / CodeQL Inefficient use of ContainsKey Note
Inefficient use of 'ContainsKey' and
indexer Error loading related location Loading
Copilot AutofixAI about 2 months ago To fix the inefficient use of string testPattern = Request.Query.ContainsKey("pattern") ? Request.Query["pattern"].ToString() ?? "aaa" : "aaa";with a call to
Suggested changeset
1
src/webapp01/Pages/DevSecOps7.cshtml.cs
Copilot is powered by AI and may make mistakes. Always verify output.
Refresh and try again.
Comment on lines
+27
to
+37
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public void OnGet() | |
| { | |
| // 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}"); | |
| // 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"; | |
| private string GetQueryValueOrDefault(string key, string defaultValue) | |
| { | |
| if (Request?.Query == null) | |
| { | |
| return defaultValue; | |
| } | |
| if (Request.Query.TryGetValue(key, out var value) && !string.IsNullOrEmpty(value)) | |
| { | |
| return value.ToString(); | |
| } | |
| return defaultValue; | |
| } | |
| public void OnGet() | |
| { | |
| // Log forging vulnerability - user input directly in logs | |
| string userInput = GetQueryValueOrDefault("user", "anonymous"); | |
| _logger.LogInformation($"User accessed DevSecOps7 page: {userInput}"); | |
| // Simulate getting latest news about GitHub Advanced Security | |
| LoadLatestGHASNews(); | |
| // Demonstrate potential ReDoS vulnerability | |
| string testPattern = GetQueryValueOrDefault("pattern", "aaa"); |
Copilot
AI
Jan 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inefficient use of 'ContainsKey' and indexer.
| string userInput = Request.Query.ContainsKey("user") ? Request.Query["user"].ToString() ?? "anonymous" : "anonymous"; | |
| _logger.LogInformation($"User accessed DevSecOps7 page: {userInput}"); | |
| // 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"; | |
| string userInput = Request.Query.TryGetValue("user", out var userValues) ? (userValues.ToString() ?? "anonymous") : "anonymous"; | |
| _logger.LogInformation($"User accessed DevSecOps7 page: {userInput}"); | |
| // Simulate getting latest news about GitHub Advanced Security | |
| LoadLatestGHASNews(); | |
| // Demonstrate potential ReDoS vulnerability | |
| string testPattern = Request.Query.TryGetValue("pattern", out var patternValues) ? (patternValues.ToString() ?? "aaa") : "aaa"; |
Check failure
Code scanning / CodeQL
Denial of Service from comparison of user input against expensive regex High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months 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)withnew Regex(@"^(a+)+$", RegexOptions.Compiled, TimeSpan.FromSeconds(1)).IsMatch(testPattern); - On line 94: Replace
VulnerableRegex.IsMatch(pattern)similarly. - Optionally, remove the static
VulnerableRegexif it is not needed elsewhere.
-
Copy modified line R18 -
Copy modified line R40 -
Copy modified line R94
| @@ -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) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months 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.
-
Copy modified lines R38-R39 -
Copy modified line R43
| @@ -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) | ||
| { |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months 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
testPatternby 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.
-
Copy modified lines R46-R47
| @@ -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 |
Check notice
Code scanning / CodeQL
Generic catch clause Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months 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: IftestPatternis null (but the code defaultstestPatternto "aaa", so this is unlikely, but may occur depending on refactoring).RegexParseException(added in .NET 7; in earlier versions, you'd getArgumentException): 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.
-
Copy modified line R43 -
Copy modified line R46 -
Copy modified lines R48-R51
| @@ -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 |
Check failure
Code scanning / CodeQL
Insecure SQL connection High
Connection string
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months 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.
-
Copy modified line R15
| @@ -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); |
Check notice
Code scanning / CodeQL
Generic catch clause Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months 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)withcatch (SqlException ex). - No additional using directives are needed as
Microsoft.Data.SqlClientis already imported on line 4.
Other aspects of the code remain unchanged. No new dependencies or libraries are required.
-
Copy modified line R56
| @@ -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}"); | ||
| } |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
deserializedData
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months 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.
| @@ -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
AI
Jan 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable deserializedData is assigned but never used. This appears to be unnecessary code since the LatestNews property is already populated before the serialization/deserialization. Consider removing this unused variable or using it if it was intended to demonstrate a specific vulnerability pattern.
| // Potential JSON deserialization vulnerability | |
| string jsonData = JsonConvert.SerializeObject(LatestNews); | |
| var deserializedData = JsonConvert.DeserializeObject<List<string>>(jsonData); | |
Check notice
Code scanning / CodeQL
Generic catch clause Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months 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.
-
Copy modified line R97 -
Copy modified lines R99-R100 -
Copy modified lines R102-R106
| @@ -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(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The System.Text.Json namespace is imported but never used in this file. Only Newtonsoft.Json is actually utilized. Consider removing this unused import to keep the code clean.