Revert "Prod release - Security fixes" #7062

kkartunov · 2025-02-03T13:55:10Z

Reverts #7059

src/server/services/gSheet.js

+        return this.getSheetAPI(req, res);
+      }
+      res.status(status);
+      return res.send((e.response && e.response.data) || { ...e, message: e.message });


To fix the problem, we need to ensure that detailed error information, including stack traces, is not sent to the client. Instead, we should log the detailed error information on the server and send a generic error message to the client. This can be achieved by modifying the catch blocks to log the error and send a generic message.

Modify the catch blocks in the getSheetAPI and addToSheetAPI methods to log the error details.

Send a generic error message to the client instead of the detailed error information.

src/server/services/mailchimp.js

+    const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}`, {
+      method: 'GET',
+      headers: {
+        'Content-Type': req.headers['content-type'],
+        Authorization: this.authorization,
+      },
+    });


To fix the SSRF vulnerability, we need to ensure that the user-provided values (listId and emailHash) are validated and sanitized before being used in the URL. This can be achieved by implementing a whitelist of acceptable listId values and ensuring that emailHash conforms to expected patterns (e.g., a valid MD5 hash).

Implement a whitelist for listId values.

Validate the emailHash to ensure it is a valid MD5 hash.

Modify the checkSubscription, doRegistMember, updateMember, and subscribeTags methods to include these validations.

src/server/services/mailchimp.js

+    const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members`, {
+      method: 'POST',
+      headers: {
+        'Content-Type': req.headers['content-type'],
+        Authorization: this.authorization,
+      },
+      body: formData,
+    });


To fix the problem, we need to ensure that the user input used in constructing the URL is validated against a whitelist of acceptable values. This will prevent attackers from manipulating the URL to perform SSRF attacks. Specifically, we should validate req.params.listId to ensure it matches expected patterns or values.

Create a whitelist of acceptable listId values.

Validate req.params.listId against this whitelist before using it in the URL.

If the listId is not valid, return an error response.

src/server/services/mailchimp.js

+    const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}`, {
+      method: 'PUT',
+      headers: {
+        'Content-Type': req.headers['content-type'],
+        Authorization: this.authorization,
+      },
+      body: formData,
+    });


To fix the problem, we need to validate and sanitize the user-provided values (req.params.listId and req.params.emailHash) before using them in the URL. This can be done by ensuring that these parameters conform to expected patterns or formats. For example, listId can be validated to ensure it matches a specific pattern, and emailHash can be checked to ensure it is a valid hash.

We will implement a validation function to check the format of listId and emailHash and use this function before constructing the URL. If the validation fails, we will throw an error or handle it appropriately.

src/server/services/mailchimp.js

+    const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}/tags`, {
+      method: 'POST',
+      headers: {
+        'Content-Type': req.headers['content-type'],
+        Authorization: this.authorization,
+      },
+      body: formData,
+    });


To fix the problem, we need to validate and sanitize the user-provided values (listId and emailHash) before incorporating them into the URL. This can be done by ensuring that these values conform to expected patterns or formats. For example, we can use regular expressions to validate that listId and emailHash contain only valid characters.

Validate listId and emailHash to ensure they conform to expected patterns.

Reject or sanitize any values that do not match the expected patterns.

Incorporate the validated values into the URL.

…aster-revert-reverted" This reverts commit 6d7b09e, reversing changes made to ffc3a93.

Revert "Prod release - Security fixes"

855706c

kkartunov requested review from ajefts and luizrrodrigues as code owners February 3, 2025 13:55

kkartunov merged commit 6d7b09e into master Feb 3, 2025
2 of 3 checks passed

github-advanced-security bot found potential problems Feb 3, 2025

View reviewed changes

kkartunov added a commit that referenced this pull request Feb 3, 2025

Revert "Merge pull request #7062 from topcoder-platform/revert-7059-m…

fccb115

…aster-revert-reverted" This reverts commit 6d7b09e, reversing changes made to ffc3a93.

@@ -59,4 +59,5 @@
                   }
+                  console.error("Error in getSheetAPI:", e);
                   res.status(status);
-                  return res.send((e.response && e.response.data) || { ...e, message: e.message });
+                  return res.send({ message: "An error occurred while processing your request." });
                 }
@@ -97,4 +98,5 @@
                   }
+                  console.error("Error in addToSheetAPI:", e);
                   res.status(status);
-                  return res.send((e.response && e.response.data) || { ...e, message: e.message });
+                  return res.send({ message: "An error occurred while processing your request." });
                 }

@@ -30,3 +30,5 @@
               async checkSubscription(req) {
-                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}`, {
+                const listId = this.validateListId(req.params.listId);
+                const emailHash = this.validateEmailHash(req.params.emailHash);
+                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${listId}/members/${emailHash}`, {
                   method: 'GET',
@@ -41,4 +43,5 @@
               async doRegistMember(req) {
+                const listId = this.validateListId(req.params.listId);
                 const formData = JSON.stringify(req.body);
-                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members`, {
+                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${listId}/members`, {
                   method: 'POST',
@@ -54,4 +57,6 @@
               async updateMember(req) {
+                const listId = this.validateListId(req.params.listId);
+                const emailHash = this.validateEmailHash(req.params.emailHash);
                 const formData = JSON.stringify(req.body);
-                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}`, {
+                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${listId}/members/${emailHash}`, {
                   method: 'PUT',
@@ -67,4 +72,6 @@
               async subscribeTags(req) {
+                const listId = this.validateListId(req.params.listId);
+                const emailHash = this.validateEmailHash(req.params.emailHash);
                 const formData = JSON.stringify(req.body);
-                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}/tags`, {
+                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${listId}/members/${emailHash}/tags`, {
                   method: 'POST',
@@ -110,2 +117,17 @@
               }
+              validateListId(listId) {
+                const allowedListIds = ['list1', 'list2', 'list3']; // Example whitelist
+                if (!allowedListIds.includes(listId)) {
+                  throw new Error('Invalid listId');
+                }
+                return listId;
+              }
+              validateEmailHash(emailHash) {
+                const emailHashPattern = /^[a-f0-9]{32}$/; // MD5 hash pattern
+                if (!emailHashPattern.test(emailHash)) {
+                  throw new Error('Invalid emailHash');
+                }
+                return emailHash;
+              }
             }

@@ -30,2 +30,6 @@
               async checkSubscription(req) {
+                const validListIds = ['list1', 'list2', 'list3']; // Example whitelist
+                if (!validListIds.includes(req.params.listId)) {
+                  throw new Error('Invalid listId');
+                }
                 const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}`, {
@@ -42,2 +46,6 @@
                 const formData = JSON.stringify(req.body);
+                const validListIds = ['list1', 'list2', 'list3']; // Example whitelist
+                if (!validListIds.includes(req.params.listId)) {
+                  throw new Error('Invalid listId');
+                }
                 const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members`, {
@@ -55,2 +63,6 @@
                 const formData = JSON.stringify(req.body);
+                const validListIds = ['list1', 'list2', 'list3']; // Example whitelist
+                if (!validListIds.includes(req.params.listId)) {
+                  throw new Error('Invalid listId');
+                }
                 const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}`, {
@@ -68,2 +80,6 @@
                 const formData = JSON.stringify(req.body);
+                const validListIds = ['list1', 'list2', 'list3']; // Example whitelist
+                if (!validListIds.includes(req.params.listId)) {
+                  throw new Error('Invalid listId');
+                }
                 const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}/tags`, {

@@ -24,2 +24,12 @@
+              isValidListId(listId) {
+                const listIdPattern = /^[a-zA-Z0-9_-]+$/;
+                return listIdPattern.test(listId);
+              }
+              isValidEmailHash(emailHash) {
+                const emailHashPattern = /^[a-f0-9]{32}$/;
+                return emailHashPattern.test(emailHash);
+              }
               /**
@@ -30,3 +40,7 @@
               async checkSubscription(req) {
-                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}`, {
+                const { listId, emailHash } = req.params;
+                if (!this.isValidListId(listId) || !this.isValidEmailHash(emailHash)) {
+                  throw new Error('Invalid parameters');
+                }
+                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${listId}/members/${emailHash}`, {
                   method: 'GET',
@@ -41,4 +55,8 @@
               async doRegistMember(req) {
+                const { listId } = req.params;
+                if (!this.isValidListId(listId)) {
+                  throw new Error('Invalid parameters');
+                }
                 const formData = JSON.stringify(req.body);
-                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members`, {
+                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${listId}/members`, {
                   method: 'POST',
@@ -54,4 +72,8 @@
               async updateMember(req) {
+                const { listId, emailHash } = req.params;
+                if (!this.isValidListId(listId) || !this.isValidEmailHash(emailHash)) {
+                  throw new Error('Invalid parameters');
+                }
                 const formData = JSON.stringify(req.body);
-                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}`, {
+                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${listId}/members/${emailHash}`, {
                   method: 'PUT',
@@ -67,4 +89,8 @@
               async subscribeTags(req) {
+                const { listId, emailHash } = req.params;
+                if (!this.isValidListId(listId) || !this.isValidEmailHash(emailHash)) {
+                  throw new Error('Invalid parameters');
+                }
                 const formData = JSON.stringify(req.body);
-                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}/tags`, {
+                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${listId}/members/${emailHash}/tags`, {
                   method: 'POST',

@@ -68,3 +68,14 @@
                 const formData = JSON.stringify(req.body);
-                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${req.params.listId}/members/${req.params.emailHash}/tags`, {
+                const listId = req.params.listId;
+                const emailHash = req.params.emailHash;
+                // Validate listId and emailHash
+                const listIdPattern = /^[a-zA-Z0-9_-]+$/;
+                const emailHashPattern = /^[a-f0-9]{32}$/;
+                if (!listIdPattern.test(listId) || !emailHashPattern.test(emailHash)) {
+                  throw new Error('Invalid listId or emailHash');
+                }
+                const res = await fetch(`${this.mailchimpBaseUrl}/lists/${listId}/members/${emailHash}/tags`, {
                   method: 'POST',

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert "Prod release - Security fixes" #7062

Revert "Prod release - Security fixes" #7062

kkartunov commented Feb 3, 2025

Check warning

Copilot Autofix

Check failure

Copilot Autofix

Check failure

Copilot Autofix

Check failure

Copilot Autofix

Check failure

Copilot Autofix

Revert "Prod release - Security fixes" #7062

Revert "Prod release - Security fixes" #7062

Conversation

kkartunov commented Feb 3, 2025

Check warning

Copilot Autofix

Check failure

Copilot Autofix

Check failure

Copilot Autofix

Check failure

Copilot Autofix

Check failure

Copilot Autofix