search

Yubico / yubikey-val

YubiKey OTP validation server in PHP
https://developers.yubico.com/yubikey-val
BSD 2-Clause "Simplified" License
130 stars 43 forks source link

Possible SQL injection vulnerability in ykval-synclib.php #62

Open aqlnce-af opened 4 years ago

aqlnce-af commented 4 years ago

https://github.com/Yubico/yubikey-val/blob/master/ykval-synclib.php#L94

Is this not a SQL injection vulnerability?

$res = $this->db->customQuery("SELECT id, secret FROM clients WHERE active='1' AND id='" . $client . "'");

StormTide commented 4 years ago

Looks like it could be. However, the reference implementation contains

https://github.com/Yubico/yubikey-val/blob/19345b76eea90d1cb3996296c12ae616d8151c22/ykval-verify.php#L202

https://github.com/Yubico/yubikey-val/blob/19345b76eea90d1cb3996296c12ae616d8151c22/ykval-common.php#L109

Function is_clientid validates this parameter before its included in the sql query. Probably a good idea to fix the SQL query form anyway, but the ctype_digit filter I suspect should prevent exploitability here in practice.