Tales of caution

Ye' old general discussion board. Basically, for everything that isn't covered elsewhere. Come here to shoot the breeze, shoot your mouth off, or whatever suits your fancy.
This forum is not for asking programming related questions.

Moderator: General Moderators

Post Reply
User avatar
CoderGoblin
DevNet Resident
Posts: 1425
Joined: Tue Mar 16, 2004 10:03 am
Location: Aachen, Germany

Tales of caution

Post 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
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post 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. :)
User avatar
shiznatix
DevNet Master
Posts: 2745
Joined: Tue Dec 28, 2004 5:57 pm
Location: Tallinn, Estonia
Contact:

Post by shiznatix »

never use constants (i.e. define()) for security related stuff (like usernames and passwords)
why?
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post 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.
User avatar
shiznatix
DevNet Master
Posts: 2745
Joined: Tue Dec 28, 2004 5:57 pm
Location: Tallinn, Estonia
Contact:

Post 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?
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post 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.
User avatar
shiznatix
DevNet Master
Posts: 2745
Joined: Tue Dec 28, 2004 5:57 pm
Location: Tallinn, Estonia
Contact:

Post 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!
User avatar
Benjamin
Site Administrator
Posts: 6935
Joined: Sun May 19, 2002 10:24 pm

Post 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.
User avatar
Kieran Huggins
DevNet Master
Posts: 3635
Joined: Wed Dec 06, 2006 4:14 pm
Location: Toronto, Canada
Contact:

Post 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?
User avatar
califdon
Jack of Zircons
Posts: 4484
Joined: Thu Nov 09, 2006 8:30 pm
Location: California, USA

Post 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?
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post 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
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post 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.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post 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.
User avatar
CoderGoblin
DevNet Resident
Posts: 1425
Joined: Tue Mar 16, 2004 10:03 am
Location: Aachen, Germany

Post 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
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post 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? ;)
Post Reply