Friends in Security - Code Review and Testing for Fun!

In a professional setting, it’s important to separate teams of security professionals to avoid bias. Even for personal projects, I always like getting additional eyes on things to make sure I didn’t miss something.

Full disclosure, I am not a software developer by any means. I know enough to hack around, and am pretty comfortable with scripting languages like Python. Most of my personal projects that require coding are based on Python. I do that mainly for the reduced time to prototype, some things like KegWatch I have plans to move to a different language for performance reasons.

To maintain full transparency, I can admit that even as a security professional sometimes I miss things or make mistakes in my own code. The further I have progressed in my career, the more I have gotten away from constant hands on keyboard work and have instead focused on compliance, risk management, security architecture, and systems engineering. As someone who has never spent a lot of time doing red team security, it’s important to have friends that do.

Code Review

I have been investing a lot of time lately in my KegWatch project. I have been focusing on the server side components while I wait for new hardware to arrive. This week I finished rewriting the API server (Python based of course, using Flask and Waitress) and wanted a proper security review done. One of my closest friends is also a cyber security professional, but on the other side and loves to poke and prod for vulnerabilities. I sent him a link to the repo and less than 24 hours later he’d identified 5 issues for me to look at. Two of the issues were just poor coding (uninitialized variable and a potential to index an empty list based on bad input), but three of them were XSS concerns.

What is XSS?

Cross Site Scripting (XSS) is a vulnerability in web applications that allows an attacker to inject executable code into the web app. The motive of the attacker would be to steal cookies, session data, user redirection (to malicious sites), or even modifying/altering a website. The primary defense to this is proper input sanitization. You need to validate and clean all input before processing it to avoid accidentally executing malicious code.

Examples

The best way to learn is from mistakes, so I am happy to advertise what I did wrong and how I plan to fix it!

Below is a snippet of code that enables XSS…

# Get data from the DELETE request
data = request.json


# Build the query
sql = 'DELETE FROM tbl_keg WHERE keg_id=%s'
values = (data.get('keg_id'),)

# Execute the query, make sure it worked...
if (db_query(sql,values,1)):
	log(1,'Successfully deleted keg: ' + str(data.get('keg_id')))
	
	#### BAD CODE ####
	return ('Deleted Keg: ' + data.get('keg_id'))

else:
	log(0,'No changes made to keg id: ' + str(data.get('keg_id')))

	#### BAD CODE #####
	return ('No changes made to keg id: ' + data.get('keg_id'))

The issue with the code above, is in my return lines. By returning data.get('keg_id') I am sending back the user’s input without any validation or sanitization. In testing, if you embed executable code within the DELETE request, it will execute:

{
	"keg_id":"<script>alert('xss');</script>"
}

Solution

As mentioned above, the best way to deal with this is to validate/sanitize user input. The other thing to do is not return the object directly provided by the user. Here is an example snippet of code for how I plan to validate the keg_id (which is a UUID) and any fields set for datetime:

def validate(dv,dt):
	# dv = data value | dt = data type
	if dt == 'uuid':
		try:
			uuid_obj = UUID(dv, version=4)
			return str(uuid_obj) == dv
		except Exception:
			return False
			
	elif dt == 'datetime':
		try:
			parse(dv)
			return True
		except Exception:
			return False

Given the way I have architected the application, I thought it would be best to build a single function for validation which makes it easier to implement as the API endpoints expand/change. Here is the workflow:

1. Send the request.json object from the HTTP request to the validator function
2. Check the object for all known parameters and make sure they are the correct format
3. Flag any pieces of data that aren’t correct
4. After checking the object, if anything failed return a boolean False; if all data passes, return a sanitized dictionary with the parameters

I’ll be honest, building it out this way just made the most sense to me…but that doesn’t mean it’s correct. I’ll sleep a little better tonight though knowing that my sweet, precious beer data is secure.

Final Thoughts

Some of these mistakes were pretty obvious, and to that I say…I know. I figured there would be a few unsafe things in the code, which is exactly why I aked for a second set of eyes. I’ve spent several hours over the last few days writing everything out to get to a functional state. Now that it works, it’s time to clean up and implement some best practices like data validation/sanitization and proper error handling. You can check out the code on my GitHub. Right now only the server side code is uploaded, but over the next few weeks I hope to get everything wrapped up and pushed.