Rushing to Judgement

On the project I am working on right now we recently went through a very extensive security audit, designed to find any potential vulnerabilities in the application. For the most part we came out smelling like a rose, but we did find some limited non-persistent XSS vulnerabilities.

A non-persistent XSS vulnerability is a scenario where a user can send malicious data (such as HTML and javascript) to the server, which is then redisplayed, but only to that user. It is arguable whether or not this really constitutes a threat but from a puristic standpoint, better to be safe than sorry.

The more dangerous type of XSS vulnerability is the persistent one - where the malicious data is saved to a database and potentially displayed to other users. Things like session hijacking, sending the user to a forged website, etc., are possible, and scary to think about.

The nice thing about the persistent vs. non-persistent XSS vulnerability is that the same solution applies to both - server-side input sanitation. Your application may already have input validation that prevents "special characters" from being allowed, as we did. Our problem came into play when we redisplayed the validated input for the user to correct - since we didn't sanitize the input characters, the penetration test team was able to cause javascript popups, etc., to appear. Had we been vulnerable to persistent attacks these buttons could have been shown to other users and God knows what else could have happened.

Since we are pretty late in the game with this application - planning to go live very soon - we did not want a heavy solution to our XSS vulnerability. Our app is a Mach-II based solution, providing lots of nice hooks for doing things application-wide, or targeting specific events. We decided to take a generic approach that would ensure all data, no matter where it comes from, is properly sanitized before we do anything else with it.

The solution for this was to wrap some sanitation code in the preEvent() method of a Mach-II plugin:

	<cffunction name="preEvent" returntype="void" hint="I am executed before each event-handler is processed.">
		<cfargument name="eventContext" type="MachII.framework.EventContext" />
		<cfset var event = arguments.eventContext.getCurrentEvent() />
		<cfset var fieldNames = "" />
		<cfset var item = "" />
				
		<cfif event.isArgDefined("fieldNames")>
			<cfset fieldNames = event.getArg("fieldNames")>
			<cfloop list="#fieldNames#" index="item">
				<cfset event.setArg(item, HTMLEditFormat(event.getArg(item))) />		
			</cfloop>
		</cfif>		
	</cffunction>	

The above code simply takes the available fieldname keys from the event context and wrappers every one with a call to HTMLEditFormat(), ensuring that any special characters are escaped. This is a quick and easy way to make sure the majority of XSS vulnerabilities are prevented.

Note that this doesn't take the place of field validation - we already had something in place for that. Field validation is more of a business logic thing and varies greatly by application. We also use prepared statements via cfqueryparam for any custom queries we use (we use Reactor for other queries which already parameterizes everything for us), which prevents SQL injection attacks.

Good luck with making your next application more secure!