Merge pull request #15 from conwetlab/develop

Fix Security Issues
conwetlab · Jan 8, 2016 · 5ca0540 · 5ca0540
2 parents 77f5391 + d0ed6f3
commit 5ca0540
Show file tree

Hide file tree

Showing 16 changed files with 384 additions and 74 deletions.
diff --git a/pom.xml b/pom.xml
@@ -443,7 +443,7 @@
 			<plugin>
 				<groupId>org.jacoco</groupId>
 				<artifactId>jacoco-maven-plugin</artifactId>
-				<version>0.7.2.201409121644</version>
+				<version>0.7.5.201505241946</version>
 				<executions>
 					<execution>
 						<goals>

diff --git a/src/main/java/org/fiware/apps/marketplace/bo/impl/DescriptionBoImpl.java b/src/main/java/org/fiware/apps/marketplace/bo/impl/DescriptionBoImpl.java
@@ -70,6 +70,7 @@
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.transaction.annotation.Transactional;
+import org.springframework.web.util.HtmlUtils;
 
 import com.hp.hpl.jena.shared.JenaException;
 
@@ -121,6 +122,9 @@ public void save(String storeName, Description description)
 			// Set the name based on the display name
 			description.setName(NameGenerator.getURLName(description.getDisplayName()));
 
+			// Escape HTML	
+			description.setComment(HtmlUtils.htmlEscape(description.getComment()));
+
 			// Exception is risen if the description is not valid
 			descriptionValidator.validateNewDescription(description);
 
@@ -275,7 +279,7 @@ private void update(String storeName, String descriptionName, Description update
 			}
 
 			if (updatedDescription.getComment() != null) {
-				descriptionToBeUpdated.setComment(updatedDescription.getComment());
+				descriptionToBeUpdated.setComment(HtmlUtils.htmlEscape(updatedDescription.getComment()));
 			}
 
 			// If the action is automatically performed by the system, last editor field

diff --git a/src/main/java/org/fiware/apps/marketplace/bo/impl/StoreBoImpl.java b/src/main/java/org/fiware/apps/marketplace/bo/impl/StoreBoImpl.java
@@ -61,6 +61,7 @@
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
+import org.springframework.web.util.HtmlUtils;
 
 @Service("storeBo")
 public class StoreBoImpl implements StoreBo{
@@ -183,6 +184,9 @@ public void save(Store store) throws NotAuthorizedException,
 			// Set default name based on the display name
 			store.setName(NameGenerator.getURLName(store.getDisplayName()));
 
+			// Escape HTML
+			store.setComment(HtmlUtils.htmlEscape(store.getComment()));
+
 			// Set average score to zero
 			store.setAverageScore(0);
 
@@ -231,7 +235,7 @@ public void update(String storeName, Store updatedStore) throws NotAuthorizedExc
 			}
 
 			if (updatedStore.getComment() != null) {
-				storeToBeUpdate.setComment(updatedStore.getComment());
+				storeToBeUpdate.setComment(HtmlUtils.htmlEscape(updatedStore.getComment()));
 			}
 
 			if (updatedStore.getDisplayName() != null) {

diff --git a/src/main/java/org/fiware/apps/marketplace/helpers/OfferingResolver.java b/src/main/java/org/fiware/apps/marketplace/helpers/OfferingResolver.java
@@ -64,6 +64,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.web.util.HtmlUtils;
 
 import com.hp.hpl.jena.shared.JenaException;
 
@@ -112,7 +113,8 @@ private String generateExceptionMessage(RdfHelper rdfHelper, String offeringUri,
 	 */
 	private String cleanRdfUrl(String url) {
 		// Remove '<' from the beginning and '>' from the end
-		return url != null ? url.substring(1, url.length() - 1) : "";
+		// Additionally, the URL is escaped in case it contains HTML...
+		return url != null ? HtmlUtils.htmlEscape(url.substring(1, url.length() - 1)) : "";
 	}
 
 

diff --git a/src/main/java/org/fiware/apps/marketplace/model/PriceComponent.java b/src/main/java/org/fiware/apps/marketplace/model/PriceComponent.java
@@ -50,6 +50,7 @@
 
 import org.fiware.apps.marketplace.exceptions.ParseException;
 import org.jboss.resteasy.annotations.providers.jaxb.IgnoreMediaTypes;
+import org.springframework.web.util.HtmlUtils;
 
 @Entity
 @Table(name = "price_components")
@@ -142,7 +143,7 @@ public PriceComponent(Map<String, List<Object>> rawPriceComponent, PricePlan pri
 	 * @return The first element as String. If the list is empty or null, an empty string is returned
 	 */
 	private static String getFirstStringFromObjectList(List<Object> list) {
-		return (list == null || list.isEmpty()) ? "" : (String) list.get(0);
+		return HtmlUtils.htmlEscape((list == null || list.isEmpty()) ? "" : (String) list.get(0));
 	}
 
 	@Id

diff --git a/src/main/java/org/fiware/apps/marketplace/model/PricePlan.java b/src/main/java/org/fiware/apps/marketplace/model/PricePlan.java
@@ -56,6 +56,7 @@
 import org.codehaus.jackson.annotate.JsonProperty;
 import org.fiware.apps.marketplace.exceptions.ParseException;
 import org.jboss.resteasy.annotations.providers.jaxb.IgnoreMediaTypes;
+import org.springframework.web.util.HtmlUtils;
 
 @Entity
 @Table(name = "price_plans")
@@ -92,9 +93,10 @@ public PricePlan(Map<String, List<Object>> rawPricePlan, Offering offering) thro
 					" contains a price plan without title");
 		}
 
-		this.title = title;
+		this.title = HtmlUtils.htmlEscape(title);
 		List<Object> ppDescriptions = rawPricePlan.get("description");
-		this.comment = (ppDescriptions != null && ppDescriptions.size() == 1) ? (String) ppDescriptions.get(0) : "";
+		this.comment = (ppDescriptions != null && ppDescriptions.size() == 1) ? 
+				HtmlUtils.htmlEscape((String) ppDescriptions.get(0)) : "";
 		this.offering = offering;
 		this.priceComponents = new HashSet<>();
 	}

diff --git a/src/main/java/org/fiware/apps/marketplace/rdf/RdfHelper.java b/src/main/java/org/fiware/apps/marketplace/rdf/RdfHelper.java
@@ -42,6 +42,7 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.web.util.HtmlUtils;
 
 import com.hp.hpl.jena.query.Query;
 import com.hp.hpl.jena.query.QueryExecution;
@@ -154,7 +155,7 @@ public List<String> queryLiterals(String query, String queriedVar) {
 		List<QuerySolution> solutions = this.query(query);
 
 		for (QuerySolution solution: solutions) {
-			literals.add(solution.getLiteral(queriedVar).getLexicalForm());
+			literals.add(HtmlUtils.htmlEscape(solution.getLiteral(queriedVar).getLexicalForm()));
 		}
 
 		return literals;

diff --git a/src/main/webapp/WEB-INF/functions.tld b/src/main/webapp/WEB-INF/functions.tld
@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<taglib 
+    xmlns="http://java.sun.com/xml/ns/javaee"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://java.sun.com/xml/ns/j2ee http://java.sun.com/xml/ns/j2ee/web-jsptaglibrary_2_0.xsd"
+    version="2.1">
+
+    <display-name>Custom Functions</display-name>    
+    <tlib-version>1.0</tlib-version>
+    <uri>http://fiware.org/functions</uri>
+
+    <function>
+        <name>escapeJS</name>
+        <function-class>org.apache.commons.lang3.StringEscapeUtils</function-class>
+        <function-signature>java.lang.String escapeEcmaScript(java.lang.String)</function-signature>
+    </function>
+    <function>
+        <name>unescapeHTML</name>
+        <function-class>org.apache.commons.lang3.StringEscapeUtils</function-class>
+        <function-signature>java.lang.String unescapeHtml4(java.lang.String)</function-signature>
+    </function>
+</taglib>
diff --git a/src/main/webapp/WEB-INF/views/descriptions/create-scripts.jsp b/src/main/webapp/WEB-INF/views/descriptions/create-scripts.jsp
@@ -1,4 +1,5 @@
 <%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %>
+<%@ taglib prefix="util" uri="http://fiware.org/functions" %>
 
 <c:if test="${ not empty storeList }">
 <script> app.view.stores = {<c:forEach var="store" items="${ storeList }">"${ store.name }": "${ store.displayName }",</c:forEach>}; </script>
@@ -7,7 +8,7 @@
 <script>
   app.view.descriptionCreateForm
   <c:forEach var="field" items="${ form_data }">
-    .addInitialValue("${ field.key }", "${ field.value }")
+    .addInitialValue("${ field.key }", "${ util:escapeJS(field.value) }")
   </c:forEach>
     .addErrorMessage("${ form_error.fieldName }", "${ form_error.fieldError }")
 </script>

diff --git a/src/main/webapp/WEB-INF/views/descriptions/detail-scripts.jsp b/src/main/webapp/WEB-INF/views/descriptions/detail-scripts.jsp
@@ -1,4 +1,5 @@
 <%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %>
+<%@ taglib prefix="util" uri="http://fiware.org/functions" %>
 
 <script src="${ pageContext.request.contextPath }/resources/marketplace/js/views/descriptions/detail.js"></script>
 <c:if test="${ viewName == 'detail' }">
@@ -7,17 +8,17 @@
   <script>
     app.view.descriptionUpdateForm
     <c:forEach var="field" items="${ form_data }">
-      .addInitialValue("${ field.key }", "${ field.value }")
+      .addInitialValue("${ field.key }", "${ util:escapeJS(field.value) }")
     </c:forEach>
       .addErrorMessage("${ form_error.fieldName }", "${ form_error.fieldError }")
   </script>
   </c:when>
   <c:otherwise>
   <script>
     app.view.descriptionUpdateForm
-      .addInitialValue("displayName", "${ description.displayName }")
-      .addInitialValue("url", "${ description.url }")
-      .addInitialValue("comment", "${ description.comment }")
+      .addInitialValue("displayName", "${ util:escapeJS(description.displayName) }")
+      .addInitialValue("url", "${ util:escapeJS(description.url) }")
+      .addInitialValue("comment", "${ util:escapeJS(util:unescapeHTML(description.comment)) }")
   </script>
   </c:otherwise>
 </c:choose>

diff --git a/src/main/webapp/WEB-INF/views/stores/detail-scripts.jsp b/src/main/webapp/WEB-INF/views/stores/detail-scripts.jsp
@@ -1,4 +1,5 @@
 <%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %>
+<%@ taglib prefix="util" uri="http://fiware.org/functions" %>
 
 <script> app.view.storeName = "${ store.name }"; </script>
 <c:if test="${ not empty review }">
@@ -24,17 +25,17 @@
     <script>
       app.view.storeForm
       <c:forEach var="field" items="${ form_data }">
-        .addInitialValue("${ field.key }", "${ field.value }")
+        .addInitialValue("${ field.key }", "${ util:escapeJS(field.value) }")
       </c:forEach>
         .addErrorMessage("${ form_error.fieldName }", "${ form_error.fieldError }")
     </script>
   </c:when>
   <c:when test="${ not empty store }">
     <script>
       app.view.storeForm
-        .addInitialValue("displayName", "${ store.displayName }")
-        .addInitialValue("url", "${ store.url }")
-        .addInitialValue("comment", "${ store.comment }")
+      	.addInitialValue("displayName", "${ util:escapeJS(store.displayName) }")
+      	.addInitialValue("url", "${ util:escapeJS(store.url) }")
+      	.addInitialValue("comment", "${ util:escapeJS(util:unescapeHTML(store.comment)) }")
     </script>
   </c:when>
   </c:choose>

diff --git a/src/main/webapp/WEB-INF/views/stores/form-scripts.jsp b/src/main/webapp/WEB-INF/views/stores/form-scripts.jsp
@@ -1,22 +1,23 @@
 <%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %>
+<%@ taglib prefix="util" uri="http://fiware.org/functions" %>
 
 <script src="${ pageContext.request.contextPath }/resources/marketplace/js/views/stores/form.js"></script>
 <c:choose>
   <c:when test="${ not empty form_data }">
   <script>
     app.view.storeForm
     <c:forEach var="field" items="${ form_data }">
-      .addInitialValue("${ field.key }", "${ field.value }")
+      .addInitialValue("${ field.key }", "${ util:escapeJS(field.value) }")
     </c:forEach>
       .addErrorMessage("${ form_error.fieldName }", "${ form_error.fieldError }")
   </script>
   </c:when>
   <c:when test="${ not empty store }">
   <script>
     app.view.storeForm
-      .addInitialValue("displayName", "${ store.displayName }")
-      .addInitialValue("url", "${ store.url }")
-      .addInitialValue("comment", "${ store.comment }")
+      .addInitialValue("displayName", "${ util:escapeJS(store.displayName) }")
+      .addInitialValue("url", "${ util:escapeJS(store.url) }")
+      .addInitialValue("comment", "${ util:escapeJS(store.comment) }")
   </script>
   </c:when>
 </c:choose>