I’m answering these questions in a new thread so as not to completely threadjack this one:
[url]http://www.webdeveloper.com/forum/showthread.php?302215-Login-page-using-php
Gonna answer this slightly out of order, bear with me…
[QUOTE=Sup3rkirby;1363323]
I do suppose I could have passed the table name into the execute rather than blindly sticking it in the string, but would that be any better overall?
You cannot pass a table name at execute — try it, doesn’t work! This is BY DESIGN.
Queries should be static strings separate from the DATA they are manipulating. Unless you’re writing something that is by definition “security, what’s that” like your own version of phpmyadmin, you shouldn’t be passing the table name, that should be static in the query.
The reasoning behind this is that adding ANYTHING to the query string is a potential place for a code elevation to stuff commands into the query. One of the whole reasons for prepared queries is so that cannot happen.
[QUOTE=Sup3rkirby;1363323]
I’m also curious about the pitfalls of pulling the password hash from the query. In the example PHP code this was only done to allow the distinction between a bad user and a bad password. I’ll admit I know it’s far more common to just provide an “[I]invalid username or password
If you clear after use it’s far, far better. Then it’s a non-issue; but it’s also a waste of internal bandwidth/execution since you could simply test it in SQL instead of pulling it and then testing it.
The problem is it’s far, FAR too easy to accidentally forget to erase the password once you check it — and if you leave it in the global scope a code elevation at any point in the code could have access to it, hence:
[QUOTE=Sup3rkirby;1363323]
So with the [B]getDB()
Depends on how complex your overall codebase is, how your includes are handled, how ‘trusted’ the codebase is (mods for example in systems like turdpress are untrustworthy crap in many cases), and your overall security methodology.
PHP is what is often referred to as “insecure by design” — it’s a interpreted scripting language meaning it is WAY too easy to create code elevations; that being where some bit of arbitrary code from user input is somehow able to be run. You have to put a lot of effort into preventing that AND making it so that IF an elevation occurs, it can’t do a whole lot of anything.
Some developers have this notion that once a code elevation occurs, all bets are off and you are shtupped. This is of course complete and utter nonsense if you use good scope practices. All that *****ing and moaning you hear about “don’t put stuff in the global scope” isn’t just about speed or efficiency (in some languages it’s the exact opposite!) but is about not allowing arbitrary code to have access to things they have no business having access too! For example user input parsing and template output have NO business having access to the database.
That’s a hefty chunk of why the mysql_ functions are ‘insecure’ — the connection is always in global scope so long as it’s open. You have an object handle like mysqli or PDO does, you can put that in local scope to a function and only pass or copy it to where it is needed.
It’s also why putting the SQL connection info in global scope is bad, as then any code elevation has the keys to the database. You suggested unsetting it right after use, and that’s good — but get a load of how idiotically insecure things like turdpress is, where they do the most nonsensical halfwit bull**** of putting them into DEFINE. [b]HERPAFREAKINGDERP!
But even if you unset it, if it’s in a separate directly callable file what’s to stop it from simply being called again? require_once would be a good start, but if it’s directly called by some elevation OUTSIDE your code?
The way I handle things some would call being a bit paranoid, and sure there’s some overhead to it — but it’s far, FAR more secure than what most off the shelf solutions seem to think is “security”.
First, I use the “one index to rule them all” design philosophy — which is to say that EVERY request from the user that is not on a ‘whitelist’ of allowed file extension is rerouted to a single index.php. It doesn’t matter what it is if it’s a PHP function the user is trying to access, index.php handles it.
An .htaccess for that is thus:
[code]RewriteEngine On
RewriteRule !.(gif|jpg|png|css|js|html|htm|txt|ico|zip|gz|rar|pdf|xml|wav|mp3|mp4|mpg|flv|swf|mkv|ogg|avi|webm|woff|svg|eot|ttf)$ index.php
[i]Though you’d be better off setting that in your PHP.ini and httpd.conf from a performance perspective
Anything not on that whitelist is redirected to index.php — including directories.
Most people when doing that would try to use .htaccess and rewrite rules to try and split the request into getdata — I find that to be grossly inefficient, inflexible and buggy, so instead I parse $_SERVER[‘REQUEST_URI’] for it.
Boom instant security since it is now impossible for visitors to call ANY of my .php library files directly from the web. Fishing for output, what’s that? That said even with that protection I operate on the principle that IF called directly no include/require should EVER output ANYTHING — meaning EVERYTHING should be wrapped in classes or functions.
Now, as to database password info — for a while I was playing with using debug_backtrace to restrict the access to the user configuration file to the main() procedure inside index.php, but that’s a waste of time and effort for little return since a code elevation could simply just readfile(‘settings.php’) to dump that info into the output anyways… instead I hard code it into index.php inside the main() function. Since EVERY request is routed through index.php, there’s your one unified secure point to change it from.
[code]function main() {
$db = new PDO(
‘mysql:host=localhost;dbname=cms’,
‘username’,
password’
);
// process rest of files here
} main();
Wrapping it in a function makes it local in scope. The problem is any ‘include’ or ‘require’ done inside main() effectively treat that function’s scope as global. ?
The solution to that is REALLY stupid.
[code]function safeRequire($filename) { require_once($filename); }
The extra function wrap breaks scope, so that include/require file does NOT have access to $db if we safeRequire inside main().
Now inside main we just pass $db as needed to classes and functions that should be allowed access, while NOT passing it to things that don’t.
I already mentioned one issue though — that a simple readfile could reveal it; just run readfile(‘index.php’) and it will spit out the code as plaintext. To prevent this, I create a subdirectory under the http root called “common” and lock open_basedir to it… in a .htaccess that would go thus:
[code]php_value open_basedir “C:xampphtdocsbasetestcommon”
Doing this means that files in the http root — like our index.php containing the password — cannot be included, required, readfiled, fopen, etc, etc…
your sql login is the most dangerous information you can have leak, and people really don’t put ANY effort towards preventing such leakage.