Would something like this be secure?

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

czerdrill
Forum Newbie
Posts: 7
Joined: Tue Jul 14, 2009 4:42 pm

Would something like this be secure?

Post by czerdrill »

Hi, I am kind of a novice with php and was wondering is code like this would be secure. Meaning would someone be able to access and get my database credentials or perform any other kind of attack?

Code: Select all

<?php
$con = mysql_connect("localhost","user","pass");
if (!$con)
  {
  die('Could not connect: ' . mysql_error());
  }
 
mysql_select_db("db_name", $con);
 
mysql_query("UPDATE jos_jawards_awards,jos_jawards_medals SET jos_jawards_awards.image = jos_jawards_medals.image WHERE
jos_jawards_awards.award = jos_jawards_medals.id");
mysql_close($con);
?>
Don't be too harsh if its horrific! lol... 8O And please suggest how I can secure it...
aliciadg
Forum Newbie
Posts: 16
Joined: Tue Jul 14, 2009 3:39 pm

Re: Would something like this be secure?

Post by aliciadg »

I wouldn't put your connect statement in your code. You could place it in an include file. I'm sure others will have better suggestions which I too will be watching for :)
czerdrill
Forum Newbie
Posts: 7
Joined: Tue Jul 14, 2009 4:42 pm

Re: Would something like this be secure?

Post by czerdrill »

So create another php file with the connect statement? and then include that file into this file?
aliciadg
Forum Newbie
Posts: 16
Joined: Tue Jul 14, 2009 3:39 pm

Re: Would something like this be secure?

Post by aliciadg »

Yes. For example you can create a folder called "includes" or something. In a file here place your connect statement that contains passwords and such. Then your current code would be:

Code: Select all

1.<?php
2.[b]include ('path/filename.include');[/b]
3.if (!$con)
4.  {
5.  die('Could not connect: ' . mysql_error());
6.  }
7. 
8.mysql_select_db("db_name", $con);
9. 
10.mysql_query("UPDATE jos_jawards_awards,jos_jawards_medals SET jos_jawards_awards.image = jos_jawards_medals.image WHERE
11.jos_jawards_awards.award = jos_jawards_medals.id");
12.mysql_close($con);
13.?>
Ensuring that the includes folder is secure may be an issue here. Maybe someone else can advise us on this :)
czerdrill
Forum Newbie
Posts: 7
Joined: Tue Jul 14, 2009 4:42 pm

Re: Would something like this be secure?

Post by czerdrill »

Lol, yeah exactly...but like you said what if the includes folder is not secured. Thanks for your help though, it is definitely better than what I had. Anyone know if the includes folder would be secure through this method?
aliciadg
Forum Newbie
Posts: 16
Joined: Tue Jul 14, 2009 3:39 pm

Re: Would something like this be secure?

Post by aliciadg »

If you look in your php.ini file you will see an entry for a path for include files. It is possible to place your include files into a secure folder that is referenced here so then you don't reference the path in the file that calls this include file. This makes the include file path less visible to the world. Additionally I would think you could set permissions appropriately on this folder that would allow your script to work, but keep you security info in your connect statment secure.
Just thought I'd add this to the discussion.
czerdrill
Forum Newbie
Posts: 7
Joined: Tue Jul 14, 2009 4:42 pm

Re: Would something like this be secure?

Post by czerdrill »

What type of permission would be suitable? 644? or less? and which directive controls the include file for php?

is it safe_mode_include_dir or include_path?
aliciadg
Forum Newbie
Posts: 16
Joined: Tue Jul 14, 2009 3:39 pm

Re: Would something like this be secure?

Post by aliciadg »

czerdrill wrote:What type of permission would be suitable? 644? or less? and which directive controls the include file for php?
I'm running IIS instead of apache, so I would try setting the permissions for "system" on the include path. The default when you install php I believe is inside the php folder, but since the default is always less secure I would change the path.
is it safe_mode_include_dir or include_path?
include_path
Eric!
DevNet Resident
Posts: 1146
Joined: Sun Jun 14, 2009 3:13 pm

Re: Would something like this be secure?

Post by Eric! »

You can place the included file in a folder that is outside of the WWW path structure being served by IIS or apache. This will prevent it from being accessed via some hole in the http server.
czerdrill
Forum Newbie
Posts: 7
Joined: Tue Jul 14, 2009 4:42 pm

Re: Would something like this be secure?

Post by czerdrill »

Thanks eric and alicia for you help. I ended up putting a config file in my home/etc/includes folder and called it from there. So it should be secure! You guys are the best :lol:
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: Would something like this be secure?

Post by jackpf »

Showing errors on a live server is a bit of a security risk, as it could give away vital information about file locations or server information. I wouldn't do it if I were you.
Eric!
DevNet Resident
Posts: 1146
Joined: Sun Jun 14, 2009 3:13 pm

Re: Would something like this be secure?

Post by Eric! »

Good catch jackpf. I didn't see that little nugget.

Remove things like your line 5 where you dump out mysql_error. Also 'or die' is good for debugging, but long term you should put in real error handling instead of leaving the user with no clue and a dead app.
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: Would something like this be secure?

Post by jackpf »

Yeah, just to expand once again, by using the trigger_error() function and set_error_handler() or something like that you can turn error reporting on on a dev machine, and off on the real thing.

I believe that's how the "pro's" do it anyway :P
czerdrill
Forum Newbie
Posts: 7
Joined: Tue Jul 14, 2009 4:42 pm

Re: Would something like this be secure?

Post by czerdrill »

Ah makes sense, so would something like a redirect to an error page be sufficient? Something like:

Code: Select all

if (!$con)
 {
 Redirect to some error page;
 }
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: Would something like this be secure?

Post by jackpf »

Well, that doesn't help you when you're trying to debug something. You could have been redirected for a number of reasons - mysql not installed, bad credentials, etc....

Like I said, you should use the trigger_error() function. For example:

Code: Select all

 
//in a file included in all pages, like a config file
$debug_mode = false;
if($debug_mode)
{
    ini_set('display_errors', E_ALL);
}
else
{
    ini_set('display_errors', 0);
    function error_handler($err_no)
    {
        if(strcmp($err_no, E_USER_ERROR) == 0)
        {
            die('There was an error');
        }
    }
    set_error_handler('error_handler');
}
 
//for queries/connections etc
mysql_connect('xxx', 'xxx', 'xxx') or trigger_error(mysql_error(), E_USER_ERROR);
 
That's basically an uncomplicated version of what I use.
Post Reply