On Fri, 2006-01-13 at 10:04 +0000, Alla Bezroutchko wrote:
> $_SESSION['login'] = $db->getOne("SELECT login FROM users WHERE login=?
> AND secret_answer=?", array($_POST['login'], $_POST['secret_answer']));
>
> As you suggest it takes a trusted value from the database. It is still
> does not prevent using register2.php to initialize $_SESSION['login']
> and then jump to resetpw4.php and have it use the value to change password.
Ah, much better. Sorry, that's what stood out for me. An 'unclean', user
supplied value was carried as trusted data. This example here uses your
known data from the DB as trusted data.
> Are you saying that user supplied data should never ever be stored in
> the session?
Not if it's used by application logic for critical evaluations, such as
authentication, authorization, etc. Sure, the user can supply something
like a nick name or whatever, something that only gets printed on the
web page. But applications should avoid taking user input and storing it
in the sessions object if it's used for comparisons somewhere, even if
verified. Instead, the known good value it was compared against during
the verification process should be stored. In a nut shell: User input is
in A and compared against the DB. If found/passed, store DB(A). not A.
in the session. Not sure if that makes sense. I prefer known good data
over user supplied data, even if they "appear" to be the same.
> I think it does, if 'login' session object is only ever set by the
> authentication procedure after verifying user's credentials.
That wasn't the case here :)
> I believe
> the usual approach to authentication in web applications is to check
> user's credentials and then store something in the session that
> indicates that the credentials were correct. Usually it is something
> like a user ID or an object or JavaBean storing user information. Then
> every pafge that needs to check auth just checks if the session object
> is present and valid.
Sure, that's how it's done. Your example overwrote that variable, which
I know is the point you were making. It's bad program design. While I
don't see it as being special, it is good that you raise awareness about
that.
It doesn't matter if it's a Session object of something else. The
application overwrote a previously trusted value with untrusted user
input. It doesn't have to be a session object, it could have been
something else.
What struck me the amount of trust the app placed in user supplied
values. Hence why I pressed the buzzer :)
> I work for a company that does penetration testing, vulnerability
> assessments, and stuff like that. The contracts we usually sign do not
> authorise us to apply blunt instruments to customers' web sites. :)
No, no, not the web site. It's creators ;)
Looks like you have your work cut out with that client. Good luck with
your efforts.
Cheers,
Frank
--
It is said that the Internet is a public utility. As such, it is best
compared to a sewer. A big, fat pipe with a bunch of crap sloshing
against your ports.
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Full-Disclosure - We believe in it. Charter: http://lists.grok.org.uk/full-disclosure-charter.html Hosted and sponsored by Secunia - http://secunia.com/