diff --git a/config/alfresco/web-scripts-application-context.xml b/config/alfresco/web-scripts-application-context.xml index 44b46b1fef..6ca6a51948 100644 --- a/config/alfresco/web-scripts-application-context.xml +++ b/config/alfresco/web-scripts-application-context.xml @@ -1427,24 +1427,33 @@ + ${links.protocosl.white.list} + + + .*s((\\*)|(\\[0]*)*)c((\\*)|(\\[0]*)*)r((\\*)|(\\[0]*)*)i((\\*)|(\\[0]*)*)p((\\*)|(\\[0]*)*)t((\\*)|(\\[0]*)*):.* + .*on.*\(.*\).*=.* + <#{'$'} + ^<.* + + - - + + - - - + + + - - - + + + diff --git a/source/java/org/alfresco/repo/web/scripts/links/AbstractLinksWebScript.java b/source/java/org/alfresco/repo/web/scripts/links/AbstractLinksWebScript.java index 7b20a1e0e4..eb462cefe7 100644 --- a/source/java/org/alfresco/repo/web/scripts/links/AbstractLinksWebScript.java +++ b/source/java/org/alfresco/repo/web/scripts/links/AbstractLinksWebScript.java @@ -24,6 +24,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; import org.alfresco.query.PagingRequest; import org.alfresco.repo.content.MimetypeMap; @@ -60,7 +61,7 @@ public abstract class AbstractLinksWebScript extends DeclarativeWebScript protected static final String PARAM_MESSAGE = "message"; protected static final String PARAM_ITEM = "item"; - + private static Log logger = LogFactory.getLog(AbstractLinksWebScript.class); // Injected services @@ -69,7 +70,11 @@ public abstract class AbstractLinksWebScript extends DeclarativeWebScript protected LinksService linksService; protected PersonService personService; protected ActivityService activityService; - + + private String protocolsWhiteList = "http,https,ftp,mailto"; + private ArrayList allowedProtocols; + private ArrayList xssPatterns; + public void setNodeService(NodeService nodeService) { this.nodeService = nodeService; @@ -94,6 +99,98 @@ public abstract class AbstractLinksWebScript extends DeclarativeWebScript { this.activityService = activityService; } + + public void setProtocolsWhiteList(String protocolsWhiteList) + { + this.protocolsWhiteList = protocolsWhiteList; + } + + public void setXssRegexp(ArrayList xssRegexp) + { + xssPatterns = new ArrayList<>(xssRegexp.size()); + for (String xssRegexpStr : xssRegexp) + { + xssPatterns.add(Pattern.compile(xssRegexpStr)); + } + } + + private boolean isProtocolAllowed(String protocol) + { + // will be used default protocol prefix + if (protocol.length() == 0) + { + return true; + } + + if (allowedProtocols == null) + { + allowedProtocols = new ArrayList(); + for (String delimProtocol : protocolsWhiteList.split(",")) + { + if (delimProtocol.trim().length() == 0) + { + continue; + } + allowedProtocols.add(delimProtocol.trim()); + } + } + + return allowedProtocols.contains(protocol); + } + + private boolean isPossibleXSS(String url) + { + // check for null + if (xssPatterns == null) + { + return false; + } + + boolean result = false; + for (Pattern pattern : xssPatterns) + { + if (pattern.matcher(url).matches()) + { + result = true; + } + } + return result; + } + + private boolean isUrlCorrect(String url) + { + //default behavior if url absent + if (url == null) + { + return true; + } + + if (url.trim().length() == 0 || isPossibleXSS(url)) + { + return false; + } + + int colonPos = url.indexOf(":"); + colonPos = colonPos > 0 ? colonPos : 0; + String protocol = url.substring(0, colonPos); + + boolean result = isProtocolAllowed(protocol); + //check for record host:port e.g.: localhost:8080 + if (!result) + { + String secondUrlPart = url.substring(colonPos+1); + int slashPos = secondUrlPart.indexOf("/"); + slashPos = slashPos > 0 ? slashPos : secondUrlPart.length(); + String port = secondUrlPart.substring(0, slashPos); + + Pattern p = Pattern.compile("^[0-9]*$"); + if (p.matcher(port).matches()) + { + result = true; + } + } + return result; + } protected String getOrNull(JSONObject json, String key) @@ -306,7 +403,18 @@ public abstract class AbstractLinksWebScript extends DeclarativeWebScript // Link name is optional String linkName = templateVars.get("path"); - + + //sanitise url + if (json != null) + { + String url = getOrNull(json, "url"); + if (!isUrlCorrect(url)) + { + String error = "Url not allowed"; + throw new WebScriptException(Status.STATUS_BAD_REQUEST, error); + } + } + // Have the real work done return executeImpl(site, linkName, req, json, status, cache); } diff --git a/source/java/org/alfresco/repo/web/scripts/links/LinkPut.java b/source/java/org/alfresco/repo/web/scripts/links/LinkPut.java index 7e373ac65d..ec96169ff0 100644 --- a/source/java/org/alfresco/repo/web/scripts/links/LinkPut.java +++ b/source/java/org/alfresco/repo/web/scripts/links/LinkPut.java @@ -18,10 +18,12 @@ */ package org.alfresco.repo.web.scripts.links; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.ResourceBundle; +import java.util.regex.Pattern; import org.alfresco.repo.security.permissions.AccessDeniedException; import org.alfresco.service.cmr.links.LinkInfo; @@ -41,8 +43,8 @@ public class LinkPut extends AbstractLinksWebScript { private static final String MSG_ACCESS_DENIED= "links.err.access.denied"; private static final String MSG_NOT_FOUND= "links.err.not.found"; - - @Override + + @Override protected Map executeImpl(SiteInfo site, String linkName, WebScriptRequest req, JSONObject json, Status status, Cache cache) { @@ -60,13 +62,14 @@ public class LinkPut extends AbstractLinksWebScript model.put(PARAM_MESSAGE, rb.getString(MSG_NOT_FOUND)); return model; } - - + // Get the new link details from the JSON // Update the main properties link.setTitle(getOrNull(json, "title")); link.setDescription(getOrNull(json, "description")); - link.setURL(getOrNull(json, "url")); + String url = getOrNull(json, "url"); + + link.setURL(url); // Handle internal / not internal if (json.containsKey("internal")) diff --git a/source/test-java/org/alfresco/repo/web/scripts/links/LinksRestApiTest.java b/source/test-java/org/alfresco/repo/web/scripts/links/LinksRestApiTest.java index a56b3966b3..82536d9505 100644 --- a/source/test-java/org/alfresco/repo/web/scripts/links/LinksRestApiTest.java +++ b/source/test-java/org/alfresco/repo/web/scripts/links/LinksRestApiTest.java @@ -20,6 +20,7 @@ package org.alfresco.repo.web.scripts.links; import java.util.Arrays; import java.util.Date; +import java.util.HashMap; import java.util.List; import javax.transaction.UserTransaction; @@ -538,6 +539,47 @@ public class LinksRestApiTest extends BaseWebScriptTest getLink(name, Status.STATUS_NOT_FOUND); deleteLink(name, Status.STATUS_NOT_FOUND); } + + /** + * MNT-13456 Check for XSS attack via update of link + * @throws Exception + */ + public void testXssLinks() throws Exception + { + String LINK_TITLE = "lnk" + System.currentTimeMillis(); + String LINK_URL = "http://alfresco.com"; + + HashMap mapForCheck = new HashMap(); + mapForCheck.put("http:javasc\\ript:alert('mail.ru')", Status.STATUS_BAD_REQUEST); + mapForCheck.put("javas\\0cr\\ip\\00t:alert('dd')", Status.STATUS_BAD_REQUEST); + mapForCheck.put("alfresco.my", Status.STATUS_OK); + mapForCheck.put("javascript:alert('http://somedata.html')", Status.STATUS_BAD_REQUEST); + mapForCheck.put("http://alfresco.org", Status.STATUS_OK); + mapForCheck.put("localhost:8080", Status.STATUS_OK); + mapForCheck.put("localhost:8080/share", Status.STATUS_OK); + mapForCheck.put("localhost:80A80/share", Status.STATUS_BAD_REQUEST); + mapForCheck.put("http:java\\00script:alert('XSS')", Status.STATUS_BAD_REQUEST); + mapForCheck.put("http:javas\\0cript:alert('XSS')", Status.STATUS_BAD_REQUEST); + mapForCheck.put("http:  javascript:alert('XSS')", Status.STATUS_BAD_REQUEST); + mapForCheck.put("", Status.STATUS_BAD_REQUEST); + mapForCheck.put("