Page 1 of 1

Tales of caution

Posted: Wed Oct 31, 2007 5:37 am
by CoderGoblin
At the moment I am having to update some legacy code where we were getting some prices going wrong. Not a big deal once I had finally tracked the problem down but time consuming never the less.

The problem.. Some time ago the German government changed the tax on goods (mwst, VAT in england) from 16% to 19%. In some places in the legacy code this was a variable, in others it was actually base price*1.16 and yet others got the information from a database (OK admit I added this previously as we had lots of files and I wanted a single place to need to change things). The problem we were having is that a couple of places the change wasn't implemented. I have just spent time going through the code tracking down any place where this mwst is calculated and standardising it to always use the value from the database.

My tale of caution is therefore... Always allow for changes in tax and make it easily changable in just one place. Also always process it using the same method.

Does anyone else have any tales which may be of benefit to people when designing code to avoid updates to legacy code ?

Other obvious ones include ...
.. Upgrade to php5 when creating new projects rather than use php4.
.. Make sure that register_globals is off, even for php4 legacy code
.. Don't rely on short tags as another server may not recognise them

Posted: Wed Oct 31, 2007 9:39 am
by feyd
General level security stuff:
  • always escape stuff being passed to databases
  • never use constants (i.e. define()) for security related stuff (like usernames and passwords)
  • be consistent about where you how you get your data, how that data is processed into a "safe" form from any tainted source (possibly including the file system)
  • never use filenames for includes that will not be parsed by PHP if called directly
  • never use PHP_SELF
  • never check for the (default) submit button of a form
  • never assume something exists -- functions, classes, libraries, variables.. the whole lot. Configurations change and something may suddenly disappear that you are relying on. This may potentially lead to full path disclosure to the user. i.e. badness.
  • always initialize variables
  • public properties are generally not your friend
  • never look directly up; pie may be falling. But always carry a big plate.
  • whitespace in your code is your friend
  • consistent indentation (and code-style in general) is extremely important (as is commenting almost excessively.)
I think that's enough for a bit. :)

Posted: Wed Oct 31, 2007 9:55 am
by shiznatix
never use constants (i.e. define()) for security related stuff (like usernames and passwords)
why?

Posted: Wed Oct 31, 2007 10:06 am
by feyd
shiznatix wrote:why?
If a piece of malicious code is executed they will gain access to that information since you cannot remove constants after they have been created.

Posted: Wed Oct 31, 2007 12:42 pm
by shiznatix
feyd wrote:
shiznatix wrote:why?
If a piece of malicious code is executed they will gain access to that information since you cannot remove constants after they have been created.
huh never thought about that. so it is best do do this:

Code: Select all

mysql_connect('host', 'username', 'password');

//instead of
$host = 'host';
$username = 'username';
$password = 'password';

mysql_connect($host, $username, $password);

//and also not this:
define('HOST', 'host');
define('USER', 'user');
define('PASS', 'pass');
mysql_connect(HOST, USER, PASS);
yes?

Posted: Wed Oct 31, 2007 12:46 pm
by feyd
The first is preferred. The second is fine if the values are unset() immediately afterward. The third should be avoided at all costs.

Posted: Wed Oct 31, 2007 12:55 pm
by shiznatix
feyd wrote:The first is preferred. The second is fine if the values are unset() immediately afterward. The third should be avoided at all costs.
aye makes sense. thanks!

Posted: Wed Oct 31, 2007 5:49 pm
by Benjamin
I have to disagree with not putting this data in constants. I believe this is common practice and that you have a lot worse issues if it's possible for malicious code to be placed into your application. If it's possible for malicious code to be placed into your application than it's more than likely possible they can find the username and password regardless of where it is. If another attack vector is used and an attacker obtains access to the database or filesystem, it really doesn't matter where your username and password is, because it will most likely be found or not needed.

Posted: Wed Oct 31, 2007 8:43 pm
by Kieran Huggins
They'll never find my username *or* password - I ate the paper they were written on and promptly forgot them both!

What about setting your username and password as environment variables in .htaccess or php.ini - would that not be safe?

Posted: Wed Oct 31, 2007 8:59 pm
by califdon
Kieran Huggins wrote:They'll never find my username *or* password - I ate the paper they were written on and promptly forgot them both!

What about setting your username and password as environment variables in .htaccess or php.ini - would that not be safe?
LMAO, great statement, Kieran!

I'm pretty naive on web security, so please indulge me. Wouldn't it be safer to store sensitive data outside of the server's document root? Are there some exploits that can reach outside of that? I'm thinking of things like using an MD5 encrypted file that PHP can access, but the web server cannot. Am I on the right track here?

Posted: Thu Nov 01, 2007 4:13 am
by Maugrim_The_Reaper
It depends really. The point is that PHP must know the details at some point - so the correct approach is to minimise and isolate that contact. That way your security risk is reduced. So of course the best bet is to use the username and password once - and not within a variable of any description. Just as feyd said really. I do think though it's all too easy to forget how open a system PHP really is - if anyone is capable of directly running malicious PHP code on your server then it's very hard to find a real avoidance.

So the true risk is allowing malicious code - that's the real problem to worry about. The rest is basically obscuration which decreases risk a little more. I'd probably go for environmental vars though if you can - it will not reduce risk for one application on one host, but since you can set up the variables for each VirtualHost independently the knowledge is not shared across hosts.

So:

1) Stop malicious code which is the real threat here
2) Include credentials on a per host basis using environmental variables to deepen defences
3) Do 2 from http.conf preferably

Posted: Thu Nov 01, 2007 8:53 am
by Jenk
When creating/reviewing code, always have the attitude of "If it can happen, it will happen." and never, ever make assumptions.

My Boss is notorious for making assumptions, credit due; a lot of the time he is right but, Murphy's law, when he is wrong, it has catastrophic consequences.

Posted: Thu Nov 01, 2007 11:32 am
by RobertGonzalez
  • If code can potentially change, encapsulate it. Abstract what does not change.
  • Reuse code as often as possible and practical.
  • If you are not distributing your applications, putting the secure credentials into the code where appropriate is preferred.
  • Make no assumptions, except...
  • Start off as though your site is totally exposed and penetrable, then code to fix those holes.

Posted: Sun Nov 04, 2007 6:18 pm
by CoderGoblin
Ok tips are all well and good but do you have stories where you didn't do one of your tips and it led to complications... or are you all very good and things always work :D

Posted: Mon Nov 05, 2007 11:43 am
by RobertGonzalez
The reason I can post the tips I posted is because they were not done and it led me to discover those things. But isn't that the way most of us dev types learn? ;)