1
0
mirror of https://github.com/Yubico/yubikey-val.git synced 2025-02-26 21:54:16 +01:00

Enhanced data validation to address YSA-2020-01

Co-authored-by: Marissa Nishimoto <marissa.nishimoto@yubico.com>
Co-authored-by: Gabriel Kihlman <g.kihlman@yubico.com>
Co-authored-by: Benno Rice <benno.rice@yubico.com>
Co-authored-by: Nigel Williams <nigel.williams@yubico.com>
This commit is contained in:
James Alseth 2019-11-07 10:30:34 -08:00 committed by Nigel Williams
parent 4d8b422808
commit d0e4db3245
No known key found for this signature in database
GPG Key ID: 669BF2ED84BDDE8B
8 changed files with 178 additions and 59 deletions

View File

@ -45,7 +45,11 @@ define('TS_REL_TOLERANCE', 0.3);
define('TS_ABS_TOLERANCE', 20); define('TS_ABS_TOLERANCE', 20);
define('TOKEN_LEN', 32); define('TOKEN_LEN', 32);
define('OTP_MAX_LEN', 48); // TOKEN_LEN plus public identity of 0..16 define('PUBID_MAX_LEN', 16);
define('OTP_MAX_LEN', TOKEN_LEN + PUBID_MAX_LEN);
define('NONCE_MIN_LEN', 16);
define('NONCE_MAX_LEN', 40);
define('INT32_LEN', 10);
function logdie ($logger, $str) function logdie ($logger, $str)
{ {
@ -70,6 +74,45 @@ function getHttpVal ($key, $default, $a)
return $val; return $val;
} }
// Verifies if a given string is modhex
function is_modhex($s) {
if (preg_match('/^[cbdefghijklnrtuv]+$/', $s) === 0) {
return false;
} else {
return true;
}
}
// Verifies if a given string is a valid OTP
function is_otp($otp) {
if ($otp == "") {
return false;
}
$otp_len = strlen($otp);
return $otp_len >= TOKEN_LEN && $otp_len <= OTP_MAX_LEN && is_modhex($otp);
}
// Verifies if a given string is a valid public id
function is_pubid($id) {
$id_len = strlen($id);
return $id_len >= 0 && $id_len <= PUBID_MAX_LEN && is_modhex($id);
}
// Verifies a given string is a valid nonce
function is_nonce($nonce) {
return strlen($nonce) >= NONCE_MIN_LEN
&& strlen($nonce) <= NONCE_MAX_LEN
&& ctype_alnum($nonce);
}
// Verifies if a given string is a valid client id
function is_clientid($id) {
if ($id == "0") {
return false;
}
return strlen($id) <= INT32_LEN && ctype_digit($id);
}
// Sign a http query string in the array of key-value pairs // Sign a http query string in the array of key-value pairs
// return b64 encoded hmac hash // return b64 encoded hmac hash
function sign($a, $apiKey, $logger) function sign($a, $apiKey, $logger)

View File

@ -156,10 +156,20 @@ class DbImpl extends Db
$query.= " FROM " . $table; $query.= " FROM " . $table;
if ($where!=null){ if ($where!=null){
foreach ($where as $key=>$value) { foreach ($where as $key=>$value) {
if ($key!=null) { if ($key != 'server' && !(ctype_alnum($value) || is_null($value)))
if ($value!=null) $match.= " ". $key . " = '" . $value . "' and"; {
else $match.= " ". $key . " is NULL and"; $this->myLog->log(LOG_WARNING, "findByMultiple: attempted to use non-alphanumeric in WHERE: " . $table . "." . $key . " = " . $value);
} return false;
}
elseif ($key == 'server' && !filter_var($value, FILTER_VALIDATE_URL))
{
$this->myLog->log(LOG_WARNING, "findByMultiple: attempted use invalid URL in WHERE: " . $table . "." . $key . " = " . $value);
return false;
}
if ($key!=null) {
if ($value!=null) $match.= " ". $key . " = '" . $value . "' and";
else $match.= " ". $key . " is NULL and";
}
} }
if ($match!=null) $query .= " WHERE" . $match; if ($match!=null) $query .= " WHERE" . $match;
$query=rtrim($query, "and"); $query=rtrim($query, "and");
@ -207,7 +217,17 @@ class DbImpl extends Db
if ($where!=null){ if ($where!=null){
$query.= " WHERE"; $query.= " WHERE";
foreach ($where as $key=>$value) { foreach ($where as $key=>$value) {
$query.= " ". $key . " = '" . $value . "' and"; if ($key != 'server' && !ctype_alnum($value))
{
$this->myLog->log(LOG_WARNING, "deleteByMultiple: attempted to write non-alphanumeric to the database: " . $value);
return false;
}
elseif ($key == 'server' && !filter_var($value, FILTER_VALIDATE_URL))
{
$this->myLog->log(LOG_WARNING, "deleteByMultiple: attempted to write invalid URL to the database: " . $value);
return false;
}
$query.= " ". $key . " = '" . $value . "' and";
} }
$query=rtrim($query, "and"); $query=rtrim($query, "and");
$query=rtrim($query); $query=rtrim($query);

View File

@ -153,7 +153,17 @@ class DbImpl extends Db
if ($where != NULL) if ($where != NULL)
{ {
foreach ($where as $key => $value) foreach ($where as $key => $value)
{ {
if ($key != 'server' && !(ctype_alnum($value) || is_null($value)))
{
$this->myLog->log(LOG_WARNING, "findByMultiple: attempted to use non-alphanumeric in WHERE: " . $table . "." . $key . " = " . $value);
return false;
}
elseif ($key == 'server' && !filter_var($value, FILTER_VALIDATE_URL))
{
$this->myLog->log(LOG_WARNING, "findByMultiple: attempted to use invalid URL in WHERE: " . $table . "." . $key . " = " . $value);
return false;
}
if ($key != NULL) if ($key != NULL)
{ {
if ($value != NULL) if ($value != NULL)
@ -216,7 +226,16 @@ class DbImpl extends Db
if ($where!=null){ if ($where!=null){
$query.= " WHERE"; $query.= " WHERE";
foreach ($where as $key=>$value) { foreach ($where as $key=>$value) {
$query.= " ". $key . " = '" . $value . "' and"; if ($key != 'server' && !ctype_alnum($value)) {
$this->myLog->log(LOG_WARNING, "deleteByMultiple: attempted to write non-alphanumeric to the database: " . $value);
return false;
}
elseif ($key == 'server' && !filter_var($value, FILTER_VALIDATE_URL))
{
$this->myLog->log(LOG_WARNING, "deleteByMultiple: attempted to write invalid URL to the database: " . $value);
return false;
}
$query.= " ". $key . " = '" . $value . "' and";
} }
$query=rtrim($query, "and"); $query=rtrim($query, "and");
$query=rtrim($query); $query=rtrim($query);

View File

@ -119,9 +119,22 @@ abstract class Db
*/ */
public function updateBy($table, $k, $v, $values) public function updateBy($table, $k, $v, $values)
{ {
if (!ctype_alnum($v) && !filter_var($v, FILTER_VALIDATE_URL)) {
$this->myLog->log(LOG_WARNING, "updateBy: attempted to use an invalid value: " . $v);
return false;
}
$query = ""; $query = "";
foreach ($values as $key=>$value){ foreach ($values as $key=>$value) {
if ($key != 'server' && !(ctype_alnum($value) || is_null($value))) {
$this->myLog->log(LOG_WARNING, "updateBy: attempted to write non-alphanumeric to the database: " . $table . "." . $key . " <= " . $value);
return false;
}
elseif ($key == 'server' && !filter_var($value, FILTER_VALIDATE_URL))
{
$this->myLog->log(LOG_WARNING, "updateBy: attempted to write invalid URL to the database: " . $table . "." . $key . " <= " . $value);
return false;
}
if (!is_null($value)) $query .= ' ' . $key . "='" . $value . "',"; if (!is_null($value)) $query .= ' ' . $key . "='" . $value . "',";
else $query .= ' ' . $key . '=NULL,'; else $query .= ' ' . $key . '=NULL,';
} }
@ -165,9 +178,23 @@ abstract class Db
*/ */
public function conditionalUpdateBy($table, $k, $v, $values, $condition) public function conditionalUpdateBy($table, $k, $v, $values, $condition)
{ {
if (!ctype_alnum($v) || preg_match('/^[a-zA-Z0-9><=()_ ]*$/', $condition) == 0)
{
$this->myLog->log(LOG_WARNING, "conditionalUpdateBy: attempted to use non-alphanumeric value: " . $v . " - " . $condition);
return false;
}
$query = ""; /* quiet the PHP Notice */ $query = ""; /* quiet the PHP Notice */
foreach ($values as $key=>$value){ foreach ($values as $key=>$value){
if ($key != 'server' && !is_int($value) && !ctype_alnum($value)) {
$this->myLog->log(LOG_WARNING, "conditionalUpdateBy: attempted to write non-alphanumeric to the database: " . $value);
return false;
}
elseif ($key == 'server' && !filter_var($value, FILTER_VALIDATE_URL))
{
$this->myLog->log(LOG_WARNING, "conditionalUpdateBy: attempted to write invalid URL to the database: " . $value);
return false;
}
$query = $query . " " . $key . "='" . $value . "',"; $query = $query . " " . $key . "='" . $value . "',";
} }
if (! $query) { if (! $query) {
@ -211,6 +238,24 @@ abstract class Db
{ {
$query= 'INSERT INTO ' . $table . " ("; $query= 'INSERT INTO ' . $table . " (";
foreach ($values as $key=>$value){ foreach ($values as $key=>$value){
if ($key == 'server') {
$v = filter_var($value, FILTER_VALIDATE_URL);
if (!$v) {
$this->myLog->log(LOG_WARNING, "save: bad server URL provided: " . $value);
return false;
}
$value = $v;
} else if ($key == 'info') {
if (preg_match('/[a-zA-Z0-9&_=,]+/', $value) == 0) {
$this->myLog->log(LOG_WARNING, "save: bad info string provided: " . $value);
return false;
}
} else {
if ($value != '' && !is_int($value) && !ctype_alnum($value)) {
$this->myLog->log(LOG_WARNING, "save: attempted to write non-alphanumeric to the database: " . $value);
return false;
}
}
if (!is_null($value)) $query = $query . $key . ","; if (!is_null($value)) $query = $query . $key . ",";
} }
$query = rtrim($query, ",") . ') VALUES ('; $query = rtrim($query, ",") . ') VALUES (';

View File

@ -47,7 +47,7 @@ $yk = $_REQUEST["yk"];
if (!$yk) { if (!$yk) {
logdie($myLog, "ERROR Missing parameter"); logdie($myLog, "ERROR Missing parameter");
} }
if (!($yk == "all" || preg_match("/^([cbdefghijklnrtuv]{0,16})$/", $yk))) { if (!($yk == "all" || is_pubid($yk))) {
logdie($myLog, "ERROR Unknown yk value: $yk"); logdie($myLog, "ERROR Unknown yk value: $yk");
} }
$myLog->addField('yk', $yk); $myLog->addField('yk', $yk);

View File

@ -47,7 +47,7 @@ $do = $_REQUEST["do"];
if (!$yk || !$do) { if (!$yk || !$do) {
logdie($myLog, "ERROR Missing parameter"); logdie($myLog, "ERROR Missing parameter");
} }
if (!preg_match("/^([cbdefghijklnrtuv]{0,16})$/", $yk)) { if (!is_pubid($yk)) {
logdie($myLog, "ERROR Unknown yk value: $yk"); logdie($myLog, "ERROR Unknown yk value: $yk");
} }
if ($do != "enable" && $do != "disable") { if ($do != "enable" && $do != "disable") {

View File

@ -94,10 +94,8 @@ if (! $sync->isConnected())
// at this point we should have the otp so let's add it to the logging module // at this point we should have the otp so let's add it to the logging module
$myLog->addField('otp', $syncParams['otp']); $myLog->addField('otp', $syncParams['otp']);
$sync->addField('otp', $syncParams['otp']);
// Verify correctness of numeric input parameters
// verify correctness of input parameters
foreach (array('modified','yk_counter', 'yk_use', 'yk_high', 'yk_low') as $param) foreach (array('modified','yk_counter', 'yk_use', 'yk_high', 'yk_low') as $param)
{ {
// -1 is valid except for modified // -1 is valid except for modified
@ -112,7 +110,26 @@ foreach (array('modified','yk_counter', 'yk_use', 'yk_high', 'yk_low') as $param
sendResp(S_MISSING_PARAMETER, $myLog); sendResp(S_MISSING_PARAMETER, $myLog);
} }
// Verify correctness of OTP input
if (!is_otp($syncParams['otp'])) {
$myLog->log(LOG_NOTICE, "Input parameter " . $syncParams['otp'] . " not correct");
sendResp(S_MISSING_PARAMETER, $myLog);
}
// Verify correctness of pubid input
if (!is_pubid($syncParams['yk_publicname'])) {
$myLog->log(LOG_NOTICE, "Input parameter " . $syncParams['yk_publicname'] . " not correct");
sendResp(S_MISSING_PARAMETER, $myLog);
}
// Verify correctness of nonce input
if (!is_nonce($syncParams['nonce'])) {
$myLog->log(LOG_NOTICE, "Input parameter " . $syncParams['nonce'] . " not correct");
sendResp(S_MISSING_PARAMETER, $myLog);
}
// get local counter data // get local counter data
$sync->addField('otp', $syncParams['otp']);
$yk_publicname = $syncParams['yk_publicname']; $yk_publicname = $syncParams['yk_publicname'];
if (($localParams = $sync->getLocalParams($yk_publicname)) === FALSE) if (($localParams = $sync->getLocalParams($yk_publicname)) === FALSE)
{ {

View File

@ -170,71 +170,46 @@ if ($protocol_version >= 2.0)
*/ */
/* Change default protocol "strings" to numeric values */ /* Change default protocol "strings" to numeric values */
if (isset($sl) && strcasecmp($sl, 'fast') == 0) if (isset($sl) && $sl != '') {
{ if (strcasecmp($sl, 'fast') == 0) {
$sl = $baseParams['__YKVAL_SYNC_FAST_LEVEL__']; $sl = $baseParams['__YKVAL_SYNC_FAST_LEVEL__'];
} } else if (strcasecmp($sl, 'secure') == 0) {
if (isset($sl) && strcasecmp($sl, 'secure') == 0) $sl = $baseParams['__YKVAL_SYNC_SECURE_LEVEL__'];
{ } else {
$sl = $baseParams['__YKVAL_SYNC_SECURE_LEVEL__']; $sl = intval($sl); // non-numbers return zero
} if ($sl < 1) {
if (!isset($sl) || $sl == '') $myLog->log(LOG_NOTICE, 'SL is provided but not correct');
{ sendResp(S_MISSING_PARAMETER, $myLog);
$sl = $baseParams['__YKVAL_SYNC_DEFAULT_LEVEL__']; }
} }
if ($sl && (preg_match("/^[0-9]+$/", $sl)==0 || ($sl<0 || $sl>100))) } else {
{ $sl = $baseParams['__YKVAL_SYNC_DEFAULT_LEVEL__'];
$myLog->log(LOG_NOTICE, 'SL is provided but not correct');
sendResp(S_MISSING_PARAMETER, $myLog);
} }
if (!isset($timeout) || $timeout == '') if (!isset($timeout) || $timeout == '') {
{
$timeout = $baseParams['__YKVAL_SYNC_DEFAULT_TIMEOUT__']; $timeout = $baseParams['__YKVAL_SYNC_DEFAULT_TIMEOUT__'];
} } else if (!ctype_digit($timeout)) {
if ($timeout && preg_match("/^[0-9]+$/", $timeout) == 0)
{
$myLog->log(LOG_NOTICE, 'timeout is provided but not correct'); $myLog->log(LOG_NOTICE, 'timeout is provided but not correct');
sendResp(S_MISSING_PARAMETER, $myLog); sendResp(S_MISSING_PARAMETER, $myLog);
} }
if ($otp == '') if (!is_otp($otp))
{
$myLog->log(LOG_NOTICE, 'OTP is missing');
sendResp(S_MISSING_PARAMETER, $myLog);
}
if (strlen($otp) < TOKEN_LEN || strlen($otp) > OTP_MAX_LEN)
{
$myLog->log(LOG_NOTICE, "Incorrect OTP length: $otp");
sendResp(S_BAD_OTP, $myLog);
}
if (preg_match('/^[cbdefghijklnrtuv]+$/', $otp) == 0)
{ {
$myLog->log(LOG_NOTICE, "Invalid OTP: $otp"); $myLog->log(LOG_NOTICE, "Invalid OTP: $otp");
sendResp(S_BAD_OTP, $myLog); sendResp(S_BAD_OTP, $myLog);
} }
if (preg_match("/^[0-9]+$/", $client) == 0) if (!is_clientid($client))
{ {
$myLog->log(LOG_NOTICE, 'id provided in request must be an integer'); $myLog->log(LOG_NOTICE, 'Client ID is missing or invalid');
sendResp(S_MISSING_PARAMETER, $myLog);
}
if ($client === '0')
{
$myLog->log(LOG_NOTICE, 'Client ID is missing');
sendResp(S_MISSING_PARAMETER, $myLog); sendResp(S_MISSING_PARAMETER, $myLog);
} }
if (isset($nonce) && preg_match("/^[A-Za-z0-9]+$/", $nonce) == 0) if (isset($nonce) && !is_nonce($nonce))
{ {
$myLog->log(LOG_NOTICE, 'NONCE is provided but not correct'); $myLog->log(LOG_NOTICE, 'NONCE is provided but not correct');
sendResp(S_MISSING_PARAMETER, $myLog); sendResp(S_MISSING_PARAMETER, $myLog);
} }
if (isset($nonce) && (strlen($nonce) < 16 || strlen($nonce) > 40))
{
$myLog->log(LOG_NOTICE, 'Nonce too short or too long');
sendResp(S_MISSING_PARAMETER, $myLog);
}
/** /**
* Timestamp parameter is not checked since current protocol * Timestamp parameter is not checked since current protocol