Skip to content

Commit e1d4bc9

Browse files
authored
URLs with escaped characters are being removed by XSS because of decoding during link building (#2456)
* Added escaping during link sanitization * Preserves Adobe Campaign expressions * Added new testcases
1 parent 07f72d7 commit e1d4bc9

File tree

4 files changed

+224
-108
lines changed

4 files changed

+224
-108
lines changed

bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/link/DefaultPathProcessor.java

+22-21
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import java.util.Optional;
1919

20-
import org.apache.commons.httpclient.URI;
2120
import org.apache.commons.lang3.StringUtils;
2221
import org.apache.sling.api.SlingHttpServletRequest;
2322
import org.apache.sling.api.resource.ResourceResolver;
@@ -106,33 +105,35 @@ public boolean accepts(@NotNull String path, @NotNull SlingHttpServletRequest re
106105
if (vanityConfig == VanityConfig.ALWAYS) {
107106
path = getPathOrVanityUrl(path, request.getResourceResolver());
108107
}
109-
int fragmentQueryMark = path.indexOf('#');
110-
if (fragmentQueryMark < 0) {
111-
fragmentQueryMark = path.indexOf('?');
112-
}
113-
114-
final String fragmentQuery;
115-
if (fragmentQueryMark >= 0) {
116-
fragmentQuery = path.substring(fragmentQueryMark);
117-
path = path.substring(0, fragmentQueryMark);
108+
int queryStringMark = path.indexOf('?');
109+
int fragmentMark = path.indexOf('#');
110+
final String queryString;
111+
final String fragment;
112+
if (queryStringMark >= 0 || fragmentMark >= 0) {
113+
if (queryStringMark >= 0) {
114+
if (fragmentMark >= 0) {
115+
queryString = path.substring(queryStringMark + 1, fragmentMark);
116+
fragment = path.substring(fragmentMark + 1);
117+
} else {
118+
queryString = path.substring(queryStringMark + 1);
119+
fragment = null;
120+
}
121+
} else {
122+
queryString = null;
123+
fragment = path.substring(fragmentMark + 1);
124+
}
125+
path = path.substring(0, queryStringMark >= 0 ? queryStringMark : fragmentMark);
118126
} else {
119-
fragmentQuery = null;
127+
queryString = null;
128+
fragment = null;
120129
}
130+
121131
String cp = request.getContextPath();
122132
if (!StringUtils.isEmpty(cp) && path.startsWith("/") && !path.startsWith(cp + "/")) {
123133
path = cp + path;
124134
}
125135

126-
try {
127-
final URI uri = new URI(path, false);
128-
path = uri.toString();
129-
} catch (Exception e) {
130-
LOG.error(e.getMessage());
131-
}
132-
133-
if (fragmentQuery != null) {
134-
path = path + fragmentQuery;
135-
}
136+
path = LinkUtil.escape(path, queryString, fragment);
136137
return path;
137138
}
138139

bundles/core/src/main/java/com/adobe/cq/wcm/core/components/internal/link/LinkBuilderImpl.java

+4-87
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,10 @@
1515
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
1616
package com.adobe.cq.wcm.core.components.internal.link;
1717

18-
import java.io.UnsupportedEncodingException;
19-
import java.math.BigInteger;
20-
import java.net.URLDecoder;
21-
import java.nio.charset.StandardCharsets;
22-
import java.security.SecureRandom;
23-
import java.util.Collections;
24-
import java.util.LinkedHashMap;
18+
import java.util.HashMap;
2519
import java.util.List;
2620
import java.util.Map;
27-
import java.util.HashMap;
2821
import java.util.Optional;
29-
import java.util.regex.Matcher;
30-
import java.util.regex.Pattern;
3122

3223
import org.apache.commons.lang3.StringUtils;
3324
import org.apache.commons.lang3.tuple.ImmutablePair;
@@ -47,20 +38,19 @@
4738
import com.day.cq.wcm.api.Page;
4839
import com.day.cq.wcm.api.PageManager;
4940

50-
import static com.adobe.cq.wcm.core.components.commons.link.Link.PN_LINK_URL;
5141
import static com.adobe.cq.wcm.core.components.commons.link.Link.PN_LINK_ACCESSIBILITY_LABEL;
5242
import static com.adobe.cq.wcm.core.components.commons.link.Link.PN_LINK_TARGET;
5343
import static com.adobe.cq.wcm.core.components.commons.link.Link.PN_LINK_TITLE_ATTRIBUTE;
44+
import static com.adobe.cq.wcm.core.components.commons.link.Link.PN_LINK_URL;
5445
import static com.adobe.cq.wcm.core.components.internal.Utils.resolveRedirects;
55-
import static com.adobe.cq.wcm.core.components.internal.link.LinkManagerImpl.VALID_LINK_TARGETS;
5646
import static com.adobe.cq.wcm.core.components.internal.link.LinkImpl.ATTR_ARIA_LABEL;
5747
import static com.adobe.cq.wcm.core.components.internal.link.LinkImpl.ATTR_TARGET;
5848
import static com.adobe.cq.wcm.core.components.internal.link.LinkImpl.ATTR_TITLE;
49+
import static com.adobe.cq.wcm.core.components.internal.link.LinkManagerImpl.VALID_LINK_TARGETS;
5950

6051
public class LinkBuilderImpl implements LinkBuilder {
6152

6253
private static final Logger LOGGER = LoggerFactory.getLogger(LinkBuilderImpl.class);
63-
private final static List<Pattern> PATTERNS = Collections.singletonList(Pattern.compile("(<%[=@].*?%>)"));
6454
public static final String HTML_EXTENSION = ".html";
6555

6656
SlingHttpServletRequest request;
@@ -162,12 +152,7 @@ public LinkBuilderImpl(String url, SlingHttpServletRequest req, List<PathProcess
162152
private @NotNull Link buildLink(String path, SlingHttpServletRequest request, Map<String, String> htmlAttributes) {
163153
if (StringUtils.isNotEmpty(path)) {
164154
try {
165-
// The link contain character sequences that are not well formatted and cannot be decoded, for example
166-
// Adobe Campaign expressions like: /content/path/to/page.html?recipient=<%= recipient.id %>
167-
Map<String, String> placeholders = new LinkedHashMap<>();
168-
String maskedPath = mask(path, placeholders);
169-
maskedPath = URLDecoder.decode(maskedPath, StandardCharsets.UTF_8.name());
170-
path = unmask(maskedPath, placeholders);
155+
path = LinkUtil.decode(path);
171156
} catch (Exception ex) {
172157
String message = "Failed to decode url '{}': {}";
173158
if (LOGGER.isDebugEnabled()) {
@@ -310,72 +295,4 @@ private Pair<Page, String> resolvePage(@NotNull final Page page) {
310295
return new ImmutablePair<>(resolved, getPageLinkURL(resolved));
311296
}
312297

313-
/**
314-
* Masks a given {@link String} by replacing all occurrences of {@link LinkBuilderImpl#PATTERNS} with a placeholder.
315-
* The generated placeholders are put into the given {@link Map} and can be used to unmask a {@link String} later on.
316-
* <p>
317-
* For example the given original {@link String} {@code /path/to/page.html?r=<%= recipient.id %>} will be transformed to
318-
* {@code /path/to/page.html?r=_abcd_} and the placeholder with the expression will be put into the given {@link Map}.
319-
*
320-
* @param original the original {@link String}
321-
* @param placeholders a {@link Map} the generated placeholders will be put in
322-
* @return the masked {@link String}
323-
* @see LinkBuilderImpl#unmask(String, Map)
324-
*/
325-
private static String mask(String original, Map<String, String> placeholders) {
326-
String masked = original;
327-
for (Pattern pattern : PATTERNS) {
328-
Matcher matcher = pattern.matcher(masked);
329-
while (matcher.find()) {
330-
String expression = matcher.group(1);
331-
String placeholder = newPlaceholder(masked);
332-
masked = masked.replaceFirst(Pattern.quote(expression), placeholder);
333-
placeholders.put(placeholder, expression);
334-
}
335-
}
336-
return masked;
337-
}
338-
339-
/**
340-
* Unmasks the given {@link String} by replacing the given placeholders with their original value.
341-
* <p>
342-
* For example the given masked {@link String} {@code /path/to/page.html?r=_abcd_} will be transformed to
343-
* {@code /path/to/page.html?r=<%= recipient.id %>} by replacing each of the given {@link Map}s keys with the corresponding value.
344-
*
345-
* @param masked the masked {@link String}
346-
* @param placeholders the {@link Map} of placeholders to replace
347-
* @return the unmasked {@link String}
348-
*/
349-
private static String unmask(String masked, Map<String, String> placeholders) {
350-
String unmasked = masked;
351-
for (Map.Entry<String, String> placeholder : placeholders.entrySet()) {
352-
unmasked = unmasked.replaceFirst(placeholder.getKey(), placeholder.getValue());
353-
}
354-
return unmasked;
355-
}
356-
357-
/**
358-
* Generate a new random placeholder that is not conflicting with any character sequence in the given {@link String}.
359-
* <p>
360-
* For example the given {@link String} {@code "foo"} a new random {@link String} will be returned that is not contained in the
361-
* given {@link String}. In this example the following {@link String}s will never be returned "f", "fo", "foo", "o", "oo".
362-
*
363-
* @param str the given {@link String}
364-
* @return the placeholder name
365-
*/
366-
private static String newPlaceholder(String str) {
367-
SecureRandom random = new SecureRandom();
368-
StringBuilder placeholderBuilder = new StringBuilder(5);
369-
370-
do {
371-
placeholderBuilder.setLength(0);
372-
placeholderBuilder
373-
.append("_")
374-
.append(new BigInteger(16, random).toString(16))
375-
.append("_");
376-
} while (str.contains(placeholderBuilder));
377-
378-
return placeholderBuilder.toString();
379-
}
380-
381298
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2+
~ Copyright 2023 Adobe
3+
~
4+
~ Licensed under the Apache License, Version 2.0 (the "License");
5+
~ you may not use this file except in compliance with the License.
6+
~ You may obtain a copy of the License at
7+
~
8+
~ http://www.apache.org/licenses/LICENSE-2.0
9+
~
10+
~ Unless required by applicable law or agreed to in writing, software
11+
~ distributed under the License is distributed on an "AS IS" BASIS,
12+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
~ See the License for the specific language governing permissions and
14+
~ limitations under the License.
15+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
16+
package com.adobe.cq.wcm.core.components.internal.link;
17+
18+
import java.io.UnsupportedEncodingException;
19+
import java.math.BigInteger;
20+
import java.net.URLDecoder;
21+
import java.net.URLEncoder;
22+
import java.nio.charset.StandardCharsets;
23+
import java.security.SecureRandom;
24+
import java.util.Collections;
25+
import java.util.LinkedHashMap;
26+
import java.util.List;
27+
import java.util.Map;
28+
import java.util.regex.Matcher;
29+
import java.util.regex.Pattern;
30+
31+
import org.apache.commons.httpclient.URI;
32+
import org.apache.commons.httpclient.URIException;
33+
import org.slf4j.Logger;
34+
import org.slf4j.LoggerFactory;
35+
36+
/**
37+
* Utility methods for handling links
38+
*/
39+
public class LinkUtil {
40+
41+
private final static Logger LOG = LoggerFactory.getLogger(LinkUtil.class);
42+
43+
private final static List<Pattern> PATTERNS = Collections.singletonList(Pattern.compile("(<%[=@].*?%>)"));
44+
45+
/**
46+
* Decodes and encoded or escaped URL taking care to not break Adobe Campaign expressions
47+
* like: /content/path/to/page.html?recipient=<%= recipient.id %>
48+
*
49+
* @param url The URL to decode
50+
* @return The decoded URL
51+
* @throws UnsupportedEncodingException
52+
*/
53+
public static String decode(final String url) throws UnsupportedEncodingException {
54+
// The link contain character sequences that are not well formatted and cannot be decoded, for example
55+
// Adobe Campaign expressions like: /content/path/to/page.html?recipient=<%= recipient.id %>
56+
final Map<String, String> placeholders = new LinkedHashMap<>();
57+
final String masked = LinkUtil.mask(url, placeholders);
58+
final String decoded = URLDecoder.decode(masked, StandardCharsets.UTF_8.name());
59+
final String unmasked = unmask(decoded, placeholders);
60+
return unmasked;
61+
}
62+
63+
/**
64+
* Escapes an URI based on path, query string and fragment: path?queryString#fragment
65+
*
66+
* @param path The URI path
67+
* @param queryString The URI query string
68+
* @param fragment The URI fragment
69+
* @return The escaped fragment
70+
*/
71+
public static String escape(final String path, final String queryString, final String fragment) {
72+
final Map<String, String> placeholders = new LinkedHashMap<>();
73+
final String maskedQueryString = mask(queryString, placeholders);
74+
String escaped;
75+
URI parsed;
76+
try {
77+
parsed = new URI(path, false);
78+
} catch (URIException e) {
79+
parsed = null;
80+
LOG.error(e.getMessage(), e);
81+
}
82+
try {
83+
if (parsed != null) {
84+
escaped = new URI(parsed.getScheme(), parsed.getAuthority(), parsed.getPath(), maskedQueryString, null).toString();
85+
} else {
86+
escaped = new URI(null, null, path, maskedQueryString, null).toString();
87+
}
88+
if (fragment != null) {
89+
StringBuilder sb = new StringBuilder(escaped);
90+
escaped = sb.append("#")
91+
.append(URLEncoder.encode(fragment, StandardCharsets.UTF_8.name()).replace("+", "%20"))
92+
.toString();
93+
}
94+
} catch (Exception e) {
95+
LOG.error(e.getMessage(), e);
96+
StringBuilder sb = new StringBuilder(path);
97+
if (queryString != null) {
98+
sb.append("?").append(maskedQueryString);
99+
}
100+
if (fragment != null) {
101+
sb.append("#").append(fragment);
102+
}
103+
escaped = sb.toString();
104+
}
105+
final String unmasked = LinkUtil.unmask(escaped, placeholders);
106+
return unmasked;
107+
}
108+
109+
/**
110+
* Masks a given {@link String} by replacing all occurrences of {@link LinkUtil#PATTERNS} with a placeholder.
111+
* The generated placeholders are put into the given {@link Map} and can be used to unmask a {@link String} later on.
112+
* <p>
113+
* For example the given original {@link String} {@code /path/to/page.html?r=<%= recipient.id %>} will be transformed to
114+
* {@code /path/to/page.html?r=_abcd_} and the placeholder with the expression will be put into the given {@link Map}.
115+
*
116+
* @param original the original {@link String}
117+
* @param placeholders a {@link Map} the generated placeholders will be put in
118+
* @return the masked {@link String}
119+
* @see LinkUtil#unmask(String, Map)
120+
*/
121+
private static String mask(final String original, final Map<String, String> placeholders) {
122+
if (original == null) {
123+
return null;
124+
}
125+
String masked = original;
126+
for (Pattern pattern : PATTERNS) {
127+
Matcher matcher = pattern.matcher(masked);
128+
while (matcher.find()) {
129+
String expression = matcher.group(1);
130+
String placeholder = newPlaceholder(masked);
131+
masked = masked.replaceFirst(Pattern.quote(expression), placeholder);
132+
placeholders.put(placeholder, expression);
133+
}
134+
}
135+
return masked;
136+
}
137+
138+
/**
139+
* Unmasks the given {@link String} by replacing the given placeholders with their original value.
140+
* <p>
141+
* For example the given masked {@link String} {@code /path/to/page.html?r=_abcd_} will be transformed to
142+
* {@code /path/to/page.html?r=<%= recipient.id %>} by replacing each of the given {@link Map}s keys with the corresponding value.
143+
*
144+
* @param masked the masked {@link String}
145+
* @param placeholders the {@link Map} of placeholders to replace
146+
* @return the unmasked {@link String}
147+
*/
148+
private static String unmask(final String masked, final Map<String, String> placeholders) {
149+
if (masked == null) {
150+
return null;
151+
}
152+
String unmasked = masked;
153+
for (Map.Entry<String, String> placeholder : placeholders.entrySet()) {
154+
unmasked = unmasked.replaceFirst(placeholder.getKey(), placeholder.getValue());
155+
}
156+
return unmasked;
157+
}
158+
159+
/**
160+
* Generate a new random placeholder that is not conflicting with any character sequence in the given {@link String}.
161+
* <p>
162+
* For example the given {@link String} {@code "foo"} a new random {@link String} will be returned that is not contained in the
163+
* given {@link String}. In this example the following {@link String}s will never be returned "f", "fo", "foo", "o", "oo".
164+
*
165+
* @param str the given {@link String}
166+
* @return the placeholder name
167+
*/
168+
private static String newPlaceholder(final String str) {
169+
SecureRandom random = new SecureRandom();
170+
StringBuilder placeholderBuilder = new StringBuilder(5);
171+
172+
do {
173+
placeholderBuilder.setLength(0);
174+
placeholderBuilder
175+
.append("_")
176+
.append(new BigInteger(16, random).toString(16))
177+
.append("_");
178+
} while (str.contains(placeholderBuilder));
179+
180+
return placeholderBuilder.toString();
181+
}
182+
}

0 commit comments

Comments
 (0)