4 ms·

What could the backend logic possibly be this worked?

Dinius 10y ago
Something like this: (PHP felt like the right approach here :p) if ($selectedOption == SECURITY_QUESTION) { if (isset($_POST["SecurityQuestion0"]) && isset(["SecurityQuestion1"])) { if ($_POST["SecurityQuestion0"] != $answer0 || $_POST["SecurityQuestion1"] != $answer1) { // invalid answers return; } } authenticateUser(); }
- dkopi 10y ago
  More likely along the lines of if ((isset($_POST["SecurityQuestion0"]) && $_POST["SecurityQuestion0"] != $answer0) || (isset($_POST["SecurityQuestion1"]) && $_POST["SecurityQuestion1"] != $answer1)
- Chyzwar 10y ago
  You should use !==. isset is do not handle all corner cases, it would return true for empty strings or false for NULL. You should use framework like Laravel: Input::has('key') By design type of security challenge should not be an option. API endpoint should not check for $selectedOption == SECURITY_QUESTION. In this case you still vulnerable for the same attack. You always should return something. having just return; is bad. Finally you should use something safer than PHP since mistake can cost you money.
psybin 10y ago
If there's a SMS challenge, process. If there's a question challenge, process. If no exceptions were thrown, you're authenticated.
deleted 10y ago
[deleted]
djf1 10y ago
return all([is_valid_answer(q, a) for q, a in params])
citruspi 10y ago
Hopefully not, but I've seen worse. def validate_security_questions(): if not question_0 or not question_1: raise AuthException('Invalid security questions') try: validate_security_questions(question_0, question_1) except AuthException as ex: # Todo: Present error to user pass
sinaa 10y ago
likely using the following pseudo-ish code: # possibly done using a session variable security_questions = [] # first question security_questions.push({question: answer}) # second question security_questions.push({question: answer}) forEach(security_questions as x) if(!validate_answer(x)) return false; return true;

Something like this: (PHP felt like the right approach here :p) if ($selectedOption == SECURITY_QUESTION) { if (isset($_POST["SecurityQuestion0"]) && isset(["SecurityQuestion1"])) { if ($_POST["SecurityQuestion0"] != $answer0 || $_POST["SecurityQuestion1"] != $answer1) { // invalid answers return; } } authenticateUser(); }

dkopi 10y ago
More likely along the lines of if ((isset($_POST["SecurityQuestion0"]) && $_POST["SecurityQuestion0"] != $answer0) || (isset($_POST["SecurityQuestion1"]) && $_POST["SecurityQuestion1"] != $answer1)
Chyzwar 10y ago
You should use !==. isset is do not handle all corner cases, it would return true for empty strings or false for NULL. You should use framework like Laravel: Input::has('key') By design type of security challenge should not be an option. API endpoint should not check for $selectedOption == SECURITY_QUESTION. In this case you still vulnerable for the same attack. You always should return something. having just return; is bad. Finally you should use something safer than PHP since mistake can cost you money.

psybin 10y ago

If there's a SMS challenge, process. If there's a question challenge, process. If no exceptions were thrown, you're authenticated.

deleted 10y ago

[deleted]

djf1 10y ago

return all([is_valid_answer(q, a) for q, a in params])

citruspi 10y ago

Hopefully not, but I've seen worse. def validate_security_questions(): if not question_0 or not question_1: raise AuthException('Invalid security questions') try: validate_security_questions(question_0, question_1) except AuthException as ex: # Todo: Present error to user pass

sinaa 10y ago

likely using the following pseudo-ish code: # possibly done using a session variable security_questions = [] # first question security_questions.push({question: answer}) # second question security_questions.push({question: answer}) forEach(security_questions as x) if(!validate_answer(x)) return false; return true;