Merge pull request gpc#279 from ilopmar/xss_vulnerability_grails2

Fix XSS vulnerability when rendering beans
grails-fields-plugin · May 25, 2018 · 4ab3a80 · 4ab3a80
2 parents b0899e1 + fcc42de
commit 4ab3a80
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 4 deletions.
diff --git a/grails-app/taglib/grails/plugin/formfields/FormFieldsTagLib.groovy b/grails-app/taglib/grails/plugin/formfields/FormFieldsTagLib.groovy
@@ -22,6 +22,7 @@ import org.codehaus.groovy.grails.commons.GrailsApplication
 import org.codehaus.groovy.grails.commons.GrailsDomainClass
 import org.codehaus.groovy.grails.commons.GrailsDomainClassProperty
 import org.codehaus.groovy.grails.plugins.support.aware.GrailsApplicationAware
+import org.codehaus.groovy.grails.support.encoding.CodecLookup
 import org.codehaus.groovy.grails.web.pages.FastStringWriter
 import org.codehaus.groovy.grails.web.pages.GroovyPage
 
@@ -37,6 +38,7 @@ class FormFieldsTagLib implements GrailsApplicationAware {
 	FormFieldsTemplateService formFieldsTemplateService
 	GrailsApplication grailsApplication
 	BeanPropertyAccessorFactory beanPropertyAccessorFactory
+	CodecLookup codecLookup
 
 	static defaultEncodeAs = [taglib:'raw']
 
@@ -245,7 +247,7 @@ class FormFieldsTagLib implements GrailsApplicationAware {
         def widgetFolder = attrs.remove('widget')
 
 		def propertyAccessor = resolveProperty(bean, property)
-		def model = buildModel(propertyAccessor, attrs)
+		def model = buildModel(propertyAccessor, attrs, 'HTML')
 
 		out << renderDisplayWidget(propertyAccessor, model, attrs, widgetFolder)
 	}
@@ -268,7 +270,7 @@ class FormFieldsTagLib implements GrailsApplicationAware {
         def widgetFolder = attrs.remove('widget')
 
 		def propertyAccessor = resolveProperty(bean, property)
-		def model = buildModel(propertyAccessor, attrs)
+		def model = buildModel(propertyAccessor, attrs, 'HTML')
 
 		def wrapperAttrs = [:]
 		def widgetAttrs = [:]
@@ -320,9 +322,12 @@ class FormFieldsTagLib implements GrailsApplicationAware {
 		beanPropertyAccessorFactory.accessorFor(bean, propertyPath)
 	}
 
-	private Map buildModel(BeanPropertyAccessor propertyAccessor, Map attrs) {
+	private Map buildModel(BeanPropertyAccessor propertyAccessor, Map attrs, String encoding = null) {
 		def value = attrs.containsKey('value') ? attrs.remove('value') : propertyAccessor.value
 		def valueDefault = attrs.remove('default')
+		if (value instanceof String && encoding) {
+			value = codecLookup.lookupEncoder(encoding).encode(value)
+		}
 		[
 			bean: propertyAccessor.rootBean,
 			property: propertyAccessor.pathFromRoot,

diff --git a/test/unit/grails/plugin/formfields/taglib/AbstractFormFieldsTagLibSpec.groovy b/test/unit/grails/plugin/formfields/taglib/AbstractFormFieldsTagLibSpec.groovy
@@ -16,7 +16,7 @@ abstract class AbstractFormFieldsTagLibSpec extends Specification {
 		personInstance = new Person(name: "Bart Simpson", password: "bartman", gender: Gender.Male, dateOfBirth: new Date(87, 3, 19), minor: true)
 		personInstance.address = new Address(street: "94 Evergreen Terrace", city: "Springfield", country: "USA")
 
-        productInstance = new Product(netPrice: 12.33)
+        productInstance = new Product(netPrice: 12.33, name: "<script>alert('XSS');</script>")
 	}
 
 	def cleanup() {

diff --git a/test/unit/grails/plugin/formfields/taglib/DisplayTagSpec.groovy b/test/unit/grails/plugin/formfields/taglib/DisplayTagSpec.groovy
@@ -208,4 +208,8 @@ class DisplayTagSpec extends AbstractFormFieldsTagLibSpec {
         applyTemplate('<f:display bean="personInstance" property="name" widget="widget"/>', [personInstance: personInstance]) == '<dt>Name</dt><dd>nospmiS traB</dd>'
     }
 
+    void 'f:display escapes one property to avoid XSS atacks'() {
+        expect:
+        applyTemplate('<f:display bean="productInstance" property="name"/>', [productInstance: productInstance]) == "&lt;script&gt;alert(&#39;XSS&#39;);&lt;/script&gt;"
+    }
 }
diff --git a/test/unit/grails/plugin/formfields/taglib/DisplayWidgetSpec.groovy b/test/unit/grails/plugin/formfields/taglib/DisplayWidgetSpec.groovy
@@ -59,4 +59,8 @@ class DisplayWidgetSpec extends AbstractFormFieldsTagLibSpec {
 		applyTemplate('<f:displayWidget bean="personInstance" property="name"/>', [personInstance: personInstance]) == 'Some displayWidget'
 	}
 
+	void 'f:displayWidget escapes values to avoid XSS atacks'() {
+		expect:
+		applyTemplate('<f:displayWidget bean="productInstance" property="name"/>', [productInstance: productInstance]) == "&lt;script&gt;alert(&#39;XSS&#39;);&lt;/script&gt;"
+	}
 }