Skip to content

Patchwork PR: Autofix #29

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.

Sign up for GitHub

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

Jump to bottom
Open
patched-codes wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Patchwork PR: Autofix #29

patched-codes wants to merge 1 commit into from

Conversation

@patched-codes
Copy link

@patched-codes patched-codes bot commented Apr 14, 2025 •
edited
Loading

This pull request from patched fixes 9 issues.

  • File changed: sqli/static/js/materialize.js
    sqli/static/js/materialize.js XSS vulnerability detected in sqli/static/js/materialize.js (lines 2849-2850) due to unsafe use of jQuery .html() method for tooltip text insertion. The code needs to be modified to use .text() instead of .html() to prevent potential script injection attacks.
    sqli/static/js/materialize.js Security vulnerability found in sqli/static/js/materialize.js (lines 8925-8926). The code uses string replace() method which only replaces first occurrence of "AM" and "PM", potentially leaving additional occurrences unhandled. This incomplete string replacement could lead to data handling issues.
    sqli/static/js/materialize.js Security ticket for incomplete string replacement in materialize.js (lines 3121-3122). Current implementation only replaces first occurrence of 'waves-notransition' in className, requiring update to global regex replacement for comprehensive string modification.
    sqli/static/js/materialize.js Security vulnerability in sqli/static/js/materialize.js (lines 1025-1026): String replace() method used without global flag could lead to incomplete sanitization. The code uses .replace("ms", "") which only replaces the first occurrence, potentially leaving subsequent occurrences unhandled. Recommend using regex with global flag: .replace(/ms/g, "")
    sqli/static/js/materialize.js XSS vulnerability found in sqli/static/js/materialize.js (lines 8590-8591). Code uses innerHTML to set empty content, which could potentially be exploited for XSS attacks. Recommendation is to replace with textContent for secure content handling.
    sqli/static/js/materialize.js XSS vulnerability found in sqli/static/js/materialize.js (lines 8588-8589) where innerHTML is used unsafely to insert an SVG element. This could allow arbitrary script execution. Replace with document.createElementNS() or textContent for secure element creation.
    sqli/static/js/materialize.js XSS vulnerability found in sqli/static/js/materialize.js (lines 3443-3444). Toast element uses innerHTML for message display, creating potential for XSS attacks. Replace with textContent for secure text rendering.
    sqli/static/js/materialize.js XSS vulnerability found in sqli/static/js/materialize.js (lines 395-396) due to unsafe use of innerHTML. Code uses innerHTML for IE browser detection, which could potentially allow XSS attacks. Recommend replacing with textContent or alternative safe DOM manipulation methods.
    sqli/static/js/materialize.js Security vulnerability in sqli/static/js/materialize.js (lines 3068-3071): Unsafe usage of hasOwnProperty method detected. The code directly calls hasOwnProperty on an object, which can be overridden or shadowed, potentially leading to security vulnerabilities. Fix required to use Object.prototype.hasOwnProperty.call() instead.

github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 14, 2025
View reviewed changes
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 14, 2025
View reviewed changes
@patched-codes patched-codes bot force-pushed the patchwork-autofix-master branch from c5c804d to b6732e9 Compare April 14, 2025 06:09
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 14, 2025
View reviewed changes
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 14, 2025
View reviewed changes
@patched-codes patched-codes bot force-pushed the patchwork-autofix-master branch from b6732e9 to f5c1dab Compare April 14, 2025 06:19
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 14, 2025
View reviewed changes
@patched-codes patched-codes bot force-pushed the patchwork-autofix-master branch from f5c1dab to 61999fe Compare April 14, 2025 09:42
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 14, 2025
View reviewed changes
@patched-codes patched-codes bot force-pushed the patchwork-autofix-master branch from 61999fe to e046408 Compare April 14, 2025 10:31
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 14, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 14, 2025
View reviewed changes
sqli/static/js/materialize.js
Comment on lines +2861 to +2868
var sanitized = div.innerHTML
.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '') // Remove script tags
.replace(/javascript:/gi, '') // Remove javascript: URLs
.replace(/onerror=/gi, '') // Remove onerror handlers
.replace(/onload=/gi, '') // Remove onload handlers
.replace(/onclick=/gi, '') // Remove onclick handlers
.replace(/onmouseover=/gi, '') // Remove mouseover handlers
.replace(/data-/gi, 'data-safe-'); // Namespace data attributes

Check failure

Code scanning / CodeQL

Incomplete URL scheme check High

This check does not consider data: and vbscript:.

Copilot Autofix

AI 9 months ago

To fix the problem, we need to extend the URL scheme check to include data: and vbscript: schemes in addition to javascript:. This will ensure that any URL starting with these schemes is sanitized to prevent code injection. The best way to fix this is to modify the sanitizeHTML function to include these additional checks.

We will update the sanitizeHTML function to replace data: and vbscript: schemes in addition to javascript:. This change will be made in the file sqli/static/js/materialize.js on line 2863.

Suggested changeset 1
sqli/static/js/materialize.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/sqli/static/js/materialize.js b/sqli/static/js/materialize.js
--- a/sqli/static/js/materialize.js
+++ b/sqli/static/js/materialize.js
@@ -2863,2 +2863,4 @@
               .replace(/javascript:/gi, '') // Remove javascript: URLs
+              .replace(/data:/gi, '') // Remove data: URLs
+              .replace(/vbscript:/gi, '') // Remove vbscript: URLs
               .replace(/onerror=/gi, '')  // Remove onerror handlers
EOF
@@ -2863,2 +2863,4 @@
.replace(/javascript:/gi, '') // Remove javascript: URLs
.replace(/data:/gi, '') // Remove data: URLs
.replace(/vbscript:/gi, '') // Remove vbscript: URLs
.replace(/onerror=/gi, '') // Remove onerror handlers
Copilot is powered by AI and may make mistakes. Always verify output.
sqli/static/js/materialize.js
Comment on lines +2861 to +2862
var sanitized = div.innerHTML
.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '') // Remove script tags

Check failure

Code scanning / CodeQL

Incomplete multi-character sanitization High

This string may still contain
<script
Loading
, which may cause an HTML element injection vulnerability.

Copilot Autofix

AI 9 months ago

To fix the problem, we need to ensure that the sanitization process is thorough and handles all potential cases of unsafe content. One effective way to achieve this is to apply the regular expression replacements repeatedly until no more replacements can be performed. This ensures that all instances of the targeted patterns are removed, even if they are nested or obfuscated.

We will modify the sanitizeHTML function to repeatedly apply the regular expression replacements until the input no longer changes. This approach will ensure that all unsafe content is fully sanitized.

Suggested changeset 1
sqli/static/js/materialize.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/sqli/static/js/materialize.js b/sqli/static/js/materialize.js
--- a/sqli/static/js/materialize.js
+++ b/sqli/static/js/materialize.js
@@ -2860,11 +2860,15 @@
             // Only allow safe tags and attributes
-            var sanitized = div.innerHTML
-              .replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '') // Remove script tags
-              .replace(/javascript:/gi, '') // Remove javascript: URLs
-              .replace(/onerror=/gi, '')  // Remove onerror handlers
-              .replace(/onload=/gi, '')   // Remove onload handlers
-              .replace(/onclick=/gi, '')   // Remove onclick handlers
-              .replace(/onmouseover=/gi, '') // Remove mouseover handlers
-              .replace(/data-/gi, 'data-safe-'); // Namespace data attributes
-            return sanitized;
+            var sanitized;
+            do {
+              sanitized = div.innerHTML;
+              div.innerHTML = sanitized
+                .replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '') // Remove script tags
+                .replace(/javascript:/gi, '') // Remove javascript: URLs
+                .replace(/onerror=/gi, '')  // Remove onerror handlers
+                .replace(/onload=/gi, '')   // Remove onload handlers
+                .replace(/onclick=/gi, '')   // Remove onclick handlers
+                .replace(/onmouseover=/gi, '') // Remove mouseover handlers
+                .replace(/data-/gi, 'data-safe-'); // Namespace data attributes
+            } while (sanitized !== div.innerHTML);
+            return div.innerHTML;
           };
EOF
@@ -2860,11 +2860,15 @@
// Only allow safe tags and attributes
var sanitized = div.innerHTML
.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '') // Remove script tags
.replace(/javascript:/gi, '') // Remove javascript: URLs
.replace(/onerror=/gi, '') // Remove onerror handlers
.replace(/onload=/gi, '') // Remove onload handlers
.replace(/onclick=/gi, '') // Remove onclick handlers
.replace(/onmouseover=/gi, '') // Remove mouseover handlers
.replace(/data-/gi, 'data-safe-'); // Namespace data attributes
return sanitized;
var sanitized;
do {
sanitized = div.innerHTML;
div.innerHTML = sanitized
.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '') // Remove script tags
.replace(/javascript:/gi, '') // Remove javascript: URLs
.replace(/onerror=/gi, '') // Remove onerror handlers
.replace(/onload=/gi, '') // Remove onload handlers
.replace(/onclick=/gi, '') // Remove onclick handlers
.replace(/onmouseover=/gi, '') // Remove mouseover handlers
.replace(/data-/gi, 'data-safe-'); // Namespace data attributes
} while (sanitized !== div.innerHTML);
return div.innerHTML;
};
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant