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

FEATURE 248 BRANCH #533

Merged
merged 8 commits into from
Nov 17, 2023
Merged

FEATURE 248 BRANCH #533

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
678451b
Remove unallowed attributes from url.
jjbennett Nov 7, 2023
c07e44b
Merge branch 'master' into feature/W-13864378-xss
jjbennett Nov 7, 2023
335b461
Only add trim when a url is present.
jjbennett Nov 7, 2023
6da7533
Fix message order and verify target attribute is allowed.
jjbennett Nov 7, 2023
6e21731
Merge branch 'feature/W-13864378-xss' of github.com:SalesforceFoundat…
jjbennett Nov 7, 2023
61c97ad
Small change to create 248 branch.
jjbennett Nov 8, 2023
54a1e8b
Merge 61c97ad7164b39f7001774f5d7a3216e9724fb3c into feature/248__W-13…
salesforce-org-metaci[bot] Nov 8, 2023
9181b50
Merge pull request #534 from SalesforceFoundation/feature/248__W-1386…
bmuniswamy Nov 9, 2023
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
32 changes: 25 additions & 7 deletions src/classes/UTIL_HtmlOutput_CTRL.cls
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
* @author Salesforce.org
* @date 2020
* @group Utilities
* @description The UTIL_HtmlOutput_CTRL is used to safely unescape any allowlisted tags and urls
* @description The UTIL_HtmlOutput_CTRL is used to safely unescape allowlisted tags and urls
*/
public with sharing class UTIL_HtmlOutput_CTRL {

Expand All @@ -53,11 +53,10 @@ public with sharing class UTIL_HtmlOutput_CTRL {

/** @description The map of allowed urls and their temporary substitution values */
private static final Map<String, String> SUBSTITUTION_BY_ALLOWED_URL = new Map<String, String> {
'<a href="/ui/setup/apex/batch/ScheduleBatchApexPage' => '|schedBatch|',
'<a href="/08e?setupid=ScheduledJobs' => '|schedJobs|',
'" target="_blank"' => '|blankTarget|',
'" target="_new"' => '|newTarget|',
'"' => '|quote|'
'<a href="/ui/setup/apex/batch/ScheduleBatchApexPage"' => '|schedBatch|',
'<a href="/08e?setupid=ScheduledJobs"' => '|schedJobs|',
'target="_blank"' => '|blankTarget|',
'target="_new"' => '|newTarget|'
};

/** @description The unescaped html passed in by the page. */
Expand Down Expand Up @@ -108,13 +107,32 @@ public with sharing class UTIL_HtmlOutput_CTRL {
}

if (hasURL) {
replacedHtml = replacedHtml.trim();
for (String allowedUrl : SUBSTITUTION_BY_ALLOWED_URL.keySet()) {
replacedHtml = replacedHtml.replace(allowedUrl, SUBSTITUTION_BY_ALLOWED_URL.get(allowedUrl));
String subbedValue = SUBSTITUTION_BY_ALLOWED_URL.get(allowedUrl);
replacedHtml = replacedHtml.replace(allowedUrl, subbedValue);
}

replacedHtml = cleanupUrl(replacedHtml, SUBSTITUTION_BY_ALLOWED_URL.values()[0]);
replacedHtml = cleanupUrl(replacedHtml, SUBSTITUTION_BY_ALLOWED_URL.values()[1]);
}

return replacedHtml;
}

private String cleanupUrl(String replacedHtml, String urlSubstitution) {
String urlAttributes = replacedHtml.substringBetween(urlSubstitution, '|emptyClose|');
if (urlAttributes == null) {
return replacedHtml;
}
if (urlAttributes.contains(SUBSTITUTION_BY_ALLOWED_URL.values()[2])) {
urlAttributes = urlAttributes.substringAfter(SUBSTITUTION_BY_ALLOWED_URL.values()[2]);
} else if (urlAttributes.contains(SUBSTITUTION_BY_ALLOWED_URL.values()[3])) {
urlAttributes = urlAttributes.substringAfter(SUBSTITUTION_BY_ALLOWED_URL.values()[3]);
}

return replacedHtml.replace(urlAttributes, '');
}

/*******************************************************************************************************************
* @description Returns the escaped html with the temporary tags and url to their original allowlisted values
Expand Down
53 changes: 43 additions & 10 deletions src/classes/UTIL_HtmlOutput_TEST.cls
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public with sharing class UTIL_HtmlOutput_TEST {
controller.unsafeHtml = html;
controller.hasUrl = false;

System.assertEquals(html, controller.getSafeHtml());
System.assertEquals(html, controller.getSafeHtml(),'Expected the bold tags with a space to be allowed.');
}

/*******************************************************************************************************
Expand All @@ -60,7 +60,7 @@ public with sharing class UTIL_HtmlOutput_TEST {
controller.unsafeHtml = html;
controller.hasUrl = false;

System.assertEquals(html, controller.getSafeHtml());
System.assertEquals(html, controller.getSafeHtml(), 'Expected the bold tags to be allowed.');
}

/*******************************************************************************************************
Expand All @@ -75,8 +75,8 @@ public with sharing class UTIL_HtmlOutput_TEST {
controller.hasUrl = false;
String actualHtml = controller.getSafeHtml();

System.assert(actualHtml.startsWith('&lt;'));
System.assertNotEquals(html, actualHtml);
System.assert(actualHtml.startsWith('&lt;'), 'Expected the img tag to be escaped.');
System.assertNotEquals(html, actualHtml, 'Expected the html to have been escaped.');
}

/*******************************************************************************************************
Expand All @@ -90,21 +90,22 @@ public with sharing class UTIL_HtmlOutput_TEST {
controller.unsafeHtml = html;
controller.hasUrl = true;

System.assertEquals(html, controller.getSafeHtml());
System.assertEquals(html, controller.getSafeHtml(), 'Expected the url to be allowed.');
}

/*******************************************************************************************************
* @description Verifies a string with allowlisted url with extra space is returned without escaping
* @description Verifies a string with allowlisted url with extra space is returned without the space
*/
@isTest
private static void shouldReturnOriginalUrlWithSpace() {
private static void shouldRemoveWhiteSpaceFromUrl() {
String html = '<a href="/ui/setup/apex/batch/ScheduleBatchApexPage" >';
String expectedHtml = '<a href="/ui/setup/apex/batch/ScheduleBatchApexPage">';
UTIL_HtmlOutput_CTRL controller = new UTIL_HtmlOutput_CTRL();

controller.unsafeHtml = html;
controller.hasUrl = true;

System.assertEquals(html, controller.getSafeHtml());
System.assertEquals(expectedHtml, controller.getSafeHtml(), 'Expected only the space to be removed from the url.');
}

/*******************************************************************************************************
Expand All @@ -119,7 +120,39 @@ public with sharing class UTIL_HtmlOutput_TEST {
controller.hasUrl = true;
String actualHtml = controller.getSafeHtml();

System.assert(actualHtml.startsWith('&lt;'));
System.assertNotEquals(html, actualHtml);
System.assert(actualHtml.startsWith('&lt;'), 'Expected the url to be escaped.');
System.assertNotEquals(html, actualHtml, 'Expected the html to be espaced.');
}

/*******************************************************************************************************
* @description Verifies unallowed attributes within the url are removed
*/
@isTest
private static void shouldReturnCleanUrl() {
String html = '<a href="/ui/setup/apex/batch/ScheduleBatchApexPage" ONMOUSEOVER=alert(document.domain)>';
String expectedHtml = '<a href="/ui/setup/apex/batch/ScheduleBatchApexPage">';
UTIL_HtmlOutput_CTRL controller = new UTIL_HtmlOutput_CTRL();

controller.unsafeHtml = html;
controller.hasUrl = true;
String actualHtml = controller.getSafeHtml();

System.assertEquals(expectedHtml, controller.getSafeHtml(), 'Expected the unallowed attributes to be removed from the url.');
}

/*******************************************************************************************************
* @description Verifies target remains in url.
*/
@isTest
private static void shouldAllowTargetInUrl() {
String html = '<a href="/ui/setup/apex/batch/ScheduleBatchApexPage" target="_blank">';
String expectedHtml = '<a href="/ui/setup/apex/batch/ScheduleBatchApexPage" target="_blank">';
UTIL_HtmlOutput_CTRL controller = new UTIL_HtmlOutput_CTRL();

controller.unsafeHtml = html;
controller.hasUrl = true;
String actualHtml = controller.getSafeHtml();

System.assertEquals(expectedHtml, controller.getSafeHtml(), 'Expected the target to remain in the url.');
}
}