Skip to content
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

fix: Mustache.escape fix #85

Merged
merged 2 commits into from
Oct 9, 2024
Merged

fix: Mustache.escape fix #85

merged 2 commits into from
Oct 9, 2024

Conversation

prakash100198
Copy link
Contributor

No description provided.

vikramdevtron
vikramdevtron previously approved these changes Oct 8, 2024
View reviewed changes
@vikramdevtron vikramdevtron self-requested a review October 8, 2024 16:29
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 9, 2024
View reviewed changes
src/destination/destinationHandlers/webhookHandler.ts
@@ -113,10 +113,10 @@
if (event.eventTypeId == EVENT_TYPE.ScoopNotification){
const date = moment(event.eventTime);
event.payload.scoopNotificationConfig.data.interceptedAt = date.unix();
jsons = Mustache.render(Mustache.escape(template), event.payload.scoopNotificationConfig.data);
jsons = Mustache.render(template, event.payload.scoopNotificationConfig.data);

Check failure

Code scanning / CodeQL

Code injection Critical

Template, which may contain code, depends on a
user-provided value
Loading
.

Copilot Autofix AI about 1 month ago

To fix the problem, we need to ensure that the template used in Mustache.render is sanitized and validated before rendering. This can be achieved by:

  1. Validating the template to ensure it does not contain any malicious code.
  2. Using a safe method to pass user input into the template rendering process.

The best way to fix this without changing existing functionality is to use context-specific escaping and validation for the template before rendering it with Mustache.

Suggested changeset 1
src/destination/destinationHandlers/webhookHandler.ts

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/destination/destinationHandlers/webhookHandler.ts b/src/destination/destinationHandlers/webhookHandler.ts
--- a/src/destination/destinationHandlers/webhookHandler.ts
+++ b/src/destination/destinationHandlers/webhookHandler.ts
@@ -115,6 +115,6 @@
                 event.payload.scoopNotificationConfig.data.interceptedAt = date.unix();
-                jsons = Mustache.render(template, event.payload.scoopNotificationConfig.data);
+                jsons = Mustache.render(this.sanitizeTemplate(template), event.payload.scoopNotificationConfig.data);
             }else {
                 let parsedEvent = this.mh.parseEventForWebhook(event as Event);
-                jsons = Mustache.render(template, parsedEvent);
+                jsons = Mustache.render(this.sanitizeTemplate(template), parsedEvent);
             }
@@ -136,2 +136,9 @@
 
+    private sanitizeTemplate(template: string): string {
+        // Implement template sanitization logic here
+        // For example, remove any potentially dangerous code or characters
+        // This is a placeholder implementation
+        return template.replace(/<script.*?>.*?<\/script>/gi, '');
+    }
+
     private saveNotificationEventSuccessLog(result: any, event: Event, p: any, setting: NotificationSettings) {
EOF
@@ -115,6 +115,6 @@
event.payload.scoopNotificationConfig.data.interceptedAt = date.unix();
jsons = Mustache.render(template, event.payload.scoopNotificationConfig.data);
jsons = Mustache.render(this.sanitizeTemplate(template), event.payload.scoopNotificationConfig.data);
}else {
let parsedEvent = this.mh.parseEventForWebhook(event as Event);
jsons = Mustache.render(template, parsedEvent);
jsons = Mustache.render(this.sanitizeTemplate(template), parsedEvent);
}
@@ -136,2 +136,9 @@

private sanitizeTemplate(template: string): string {
// Implement template sanitization logic here
// For example, remove any potentially dangerous code or characters
// This is a placeholder implementation
return template.replace(/<script.*?>.*?<\/script>/gi, '');
}

private saveNotificationEventSuccessLog(result: any, event: Event, p: any, setting: NotificationSettings) {
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
src/destination/destinationHandlers/webhookHandler.ts
}else {
let parsedEvent = this.mh.parseEventForWebhook(event as Event);
jsons = Mustache.render(Mustache.escape(template), parsedEvent);
jsons = Mustache.render(template, parsedEvent);

Check failure

Code scanning / CodeQL

Code injection Critical

Template, which may contain code, depends on a
user-provided value
Loading
.

Copilot Autofix AI about 1 month ago

To fix the problem, we need to ensure that the template used in Mustache.render is sanitized and validated before rendering. This can be achieved by escaping any potentially dangerous characters in the user-provided input. Additionally, we should validate the structure of the template to ensure it conforms to expected patterns.

  1. Sanitize the template: Use a library like DOMPurify to sanitize the template string.
  2. Validate the template: Ensure the template conforms to expected patterns and does not contain any unexpected or dangerous content.
Suggested changeset 2
src/destination/destinationHandlers/webhookHandler.ts

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/destination/destinationHandlers/webhookHandler.ts b/src/destination/destinationHandlers/webhookHandler.ts
--- a/src/destination/destinationHandlers/webhookHandler.ts
+++ b/src/destination/destinationHandlers/webhookHandler.ts
@@ -20,2 +20,3 @@
 import Mustache from 'mustache';
+import DOMPurify from 'dompurify';
 import { EventLogBuilder } from "../../common/eventLogBuilder";
@@ -115,6 +116,8 @@
                 event.payload.scoopNotificationConfig.data.interceptedAt = date.unix();
-                jsons = Mustache.render(template, event.payload.scoopNotificationConfig.data);
+                const sanitizedTemplate = DOMPurify.sanitize(template);
+                jsons = Mustache.render(sanitizedTemplate, event.payload.scoopNotificationConfig.data);
             }else {
                 let parsedEvent = this.mh.parseEventForWebhook(event as Event);
-                jsons = Mustache.render(template, parsedEvent);
+                const sanitizedTemplate = DOMPurify.sanitize(template);
+                jsons = Mustache.render(sanitizedTemplate, parsedEvent);
             }
EOF
@@ -20,2 +20,3 @@
import Mustache from 'mustache';
import DOMPurify from 'dompurify';
import { EventLogBuilder } from "../../common/eventLogBuilder";
@@ -115,6 +116,8 @@
event.payload.scoopNotificationConfig.data.interceptedAt = date.unix();
jsons = Mustache.render(template, event.payload.scoopNotificationConfig.data);
const sanitizedTemplate = DOMPurify.sanitize(template);
jsons = Mustache.render(sanitizedTemplate, event.payload.scoopNotificationConfig.data);
}else {
let parsedEvent = this.mh.parseEventForWebhook(event as Event);
jsons = Mustache.render(template, parsedEvent);
const sanitizedTemplate = DOMPurify.sanitize(template);
jsons = Mustache.render(sanitizedTemplate, parsedEvent);
}
package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/package.json b/package.json
--- a/package.json
+++ b/package.json
@@ -41,3 +41,4 @@
         "typeorm": "0.3.17",
-        "winston": "^3.2.1"
+        "winston": "^3.2.1",
+        "dompurify": "^3.1.7"
     },
EOF
@@ -41,3 +41,4 @@
"typeorm": "0.3.17",
"winston": "^3.2.1"
"winston": "^3.2.1",
"dompurify": "^3.1.7"
},
This fix introduces these dependencies
Package Version Security advisories
dompurify (npm) 3.1.7 None
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@prakash100198 prakash100198 merged commit 133c887 into main Oct 9, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shivam-nagar23 Shivam-nagar23 Shivam-nagar23 approved these changes

@vivek-devtron vivek-devtron vivek-devtron approved these changes

@vikramdevtron vikramdevtron Awaiting requested review from vikramdevtron

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@prakash100198 @vikramdevtron @vivek-devtron @Shivam-nagar23