Page 1 of 1

Are there any security holes in this script?

Posted: Tue May 11, 2004 6:12 am
by connectsunny
<html>
<head>
<title>My Photo Album (also known as the "Swiss Cheese")</title>
</head>
<body>
<h1>My Photo Album</h1>
<?php
$adminPassword = "thebigboss"; // change the password anyway!

// try to authenticate
if ($_POST["password"] == $adminPassword) {
$isAdmin = true;
}
$dbServer = "localhost";
$database = "photoalbum";
$dbUser = "ulrich";
$dbPassword = "hotshots";
$connection = mysql_connect($dbServer, $dbUser, $dbPassword);
mysql_select_db($database,$connection);
if (($isAdmin) && ($_POST["photoNumber"] != "")) {
if ($_POST["photoNumber"] == "new") {
// insert a new photo into the database
if (is_uploaded_file($_FILES["photoFile"]["tmp_name"])) {
// new photo uploaded, get file name and save photo
$photoCaption = $_POST["photoCaption"];
$photoName = basename($_FILES["photoFile"]["name"]);
copy($_FILES["photoFile"]["tmp_name"],"images/$photoName");
$query = "insert into photos (caption, filename)
values (\"".$photoCaption."\",
\"".$photoName."\")";
mysql_query($query,$connection);
}
}
else
{
// update data for an existing photo
$photoNumber = $_POST["photoNumber"];
$photoCaption = $_POST["photoCaption"];
if (is_uploaded_file($_FILES["photoFile"]["tmp_name"])) {
// different photo uploaded, get file name and save photo
$photoName = basename($_FILES["photoFile"]["name"]);
copy($_FILES["photoFile"]["tmp_name"],"images/$photoName");
$query = "update photos
set caption = \"".$photoCaption."\",
filename = \"".$photoName."\"
where id = ".$photoNumber;
mysql_query($query,$connection);
}
else
{
$query = "update photos
set caption = \"".$photoCaption."\"
where id = ".$photoNumber;
mysql_query($query,$connection);
}
}
}
$query = "select * from photos";
$result = mysql_query($query,$connection);
while ($record = mysql_fetch_array($result)) {
echo "<tr><td>";
echo "<img src=\"images/".$record["filename"]."\">";
echo "</td><td>";
if ($isAdmin) {
// display form
echo "<form action=\"photoalbum.php\" method=\"post\" enctype=\"multipart/form-data\">";
echo "<input type=\"hidden\" name=\"photoNumber\" ";
echo "value=\"".$record["id"]."\">";
echo "<input type=\"hidden\" name=\"password\" ";
echo "value=\"".$_POST["password"]."\">";
echo "Caption:<br>";
echo "<input type=\"text\" name=\"photoCaption\" size=\"60\"";
echo "value=\"".$record["caption"]."\"><br>";
echo "File:<br>";
echo "<input type=\"file\" name=\"photoFile\">";
echo "<input type=\"submit\" value=\"Update photo\">";
echo "</form>";
}
else
{
// display caption only
echo $record["caption"];
}
echo "</td></tr>";
}

// Now there's only one thing left to do:
// - if the user is logged in as admin, display a form to add another photo
// - in all other cases display the login form (in which the admin can enter the password)
//
echo "<tr><td colspan=\"2\">";
if ($isAdmin) {
// display "Add photo" form
echo "<h3>Add a new photo</h3>";
echo "<form action=\"photoalbum.php\" method=\"post\" enctype=\"multipart/form-data\">";
echo "<input type=\"hidden\" name=\"photoNumber\" ";
echo "value=\"new\">";
echo "<input type=\"hidden\" name=\"password\" ";
echo "value=\"".$_POST["password"]."\">";
echo "Caption:<br>";
echo "<input type=\"text\" name=\"photoCaption\" size=\"60\"><br>";
echo "File:<br>";
echo "<input type=\"file\" name=\"photoFile\"><br>";
echo "<input type=\"submit\" value=\"Add photo\">";
echo "</form>";
}
else
{
// display login form
echo "<h3>Enter the admin password to add a photo</h3>";
echo "<form action=\"photoalbum.php\" method=\"post\">";
echo "Password:";
echo "<input type=\"password\" name=\"password\">";
echo "<input type=\"submit\" value=\"login\">";
echo "</form>";
}
echo "</td></tr>";


?>
</table>
</body>
</html>

PLEASE HELP ME ...... WHAT R THE MOST OBVIOUS HOLES ? AND HOW MANY ARE THERE ROUGHLY, I AM A NOVICE IN php plzzzzzzz help or suggest some readings plzzzzzzzzz .... i n dire need

Posted: Tue May 11, 2004 6:23 am
by magicrobotmonkey
Let me make it easier on the eyes...

Code: Select all

&lt;html&gt; 
&lt;head&gt; 
&lt;title&gt;My Photo Album (also known as the "Swiss Cheese")&lt;/title&gt; 
&lt;/head&gt; 
&lt;body&gt; 
&lt;h1&gt;My Photo Album&lt;/h1&gt; 
&lt;?php 
$adminPassword = "thebigboss"; // change the password anyway! 

// try to authenticate 
if ($_POST&#1111;"password"] == $adminPassword) { 
  $isAdmin = true; 
} 

$dbServer = "localhost"; 
$database = "photoalbum"; 
$dbUser = "ulrich"; 
$dbPassword = "hotshots"; 
$connection = mysql_connect($dbServer, $dbUser, $dbPassword); 
mysql_select_db($database,$connection); 
if (($isAdmin) &amp;&amp; ($_POST&#1111;"photoNumber"] != "")) { 
  if ($_POST&#1111;"photoNumber"] == "new") { 
    // insert a new photo into the database 
    if (is_uploaded_file($_FILES&#1111;"photoFile"]&#1111;"tmp_name"])) { 
      // new photo uploaded, get file name and save photo 
      $photoCaption = $_POST&#1111;"photoCaption"]; 
      $photoName = basename($_FILES&#1111;"photoFile"]&#1111;"name"]); 
      copy($_FILES&#1111;"photoFile"]&#1111;"tmp_name"],"images/$photoName"); 
      $query = "insert into photos (caption, filename) 
                values ("".$photoCaption."", "".$photoName."")"; 
      mysql_query($query,$connection); 
    } 
  } 
  else 
 { 
    // update data for an existing photo 
    $photoNumber = $_POST&#1111;"photoNumber"]; 
    $photoCaption = $_POST&#1111;"photoCaption"]; 
    if (is_uploaded_file($_FILES&#1111;"photoFile"]&#1111;"tmp_name"])) { 
      // different photo uploaded, get file name and save photo 
      $photoName = basename($_FILES&#1111;"photoFile"]&#1111;"name"]); 
      copy($_FILES&#1111;"photoFile"]&#1111;"tmp_name"],"images/$photoName"); 
      $query = "update photos 
      set caption = "".$photoCaption."", 
      filename = "".$photoName."" 
      where id = ".$photoNumber; 
      mysql_query($query,$connection); 
    } 
    else 
   { 
      $query = "update photos 
      set caption = "".$photoCaption."" 
      where id = ".$photoNumber; 
      mysql_query($query,$connection); 
    } 
  } 
} 
$query = "select * from photos"; 
$result = mysql_query($query,$connection); 
while ($record = mysql_fetch_array($result)) { 
  echo "&lt;tr&gt;&lt;td&gt;"; 
  echo "&lt;img src="images/".$record&#1111;"filename"].""&gt;"; 
  echo "&lt;/td&gt;&lt;td&gt;"; 
  if ($isAdmin) { 
    // display form 
    echo "&lt;form action="photoalbum.php" method="post"        
enctype="multipart/form-data"&gt;"; 
  echo "&lt;input type="hidden" name="photoNumber" "; 
  echo "value="".$record&#1111;"id"].""&gt;"; 
  echo "&lt;input type="hidden" name="password" "; 
  echo "value="".$_POST&#1111;"password"].""&gt;"; 
  echo "Caption:&lt;br&gt;"; 
  echo "&lt;input type="text" name="photoCaption" size="60""; 
  echo "value="".$record&#1111;"caption"].""&gt;&lt;br&gt;"; 
  echo "File:&lt;br&gt;"; 
  echo "&lt;input type="file" name="photoFile"&gt;"; 
  echo "&lt;input type="submit" value="Update photo"&gt;"; 
  echo "&lt;/form&gt;"; 
} 
else 
{ 
  // display caption only 
  echo $record&#1111;"caption"]; 
} 
echo "&lt;/td&gt;&lt;/tr&gt;"; 
} 

// Now there's only one thing left to do: 
// - if the user is logged in as admin, display a form to add another photo 
// - in all other cases display the login form (in which the admin can enter the password) 
// 
echo "&lt;tr&gt;&lt;td colspan="2"&gt;"; 
if ($isAdmin) { 
  // display "Add photo" form 
  echo "&lt;h3&gt;Add a new photo&lt;/h3&gt;"; 
  echo "&lt;form action="photoalbum.php" method="post"   enctype="multipart/form-data"&gt;"; 
  echo "&lt;input type="hidden" name="photoNumber" "; 
  echo "value="new"&gt;"; 
  echo "&lt;input type="hidden" name="password" "; 
  echo "value="".$_POST&#1111;"password"].""&gt;"; 
  echo "Caption:&lt;br&gt;"; 
  echo "&lt;input type="text" name="photoCaption" size="60"&gt;&lt;br&gt;"; 
  echo "File:&lt;br&gt;"; 
  echo "&lt;input type="file" name="photoFile"&gt;&lt;br&gt;"; 
  echo "&lt;input type="submit" value="Add photo"&gt;"; 
  echo "&lt;/form&gt;"; 
} 
else 
{ 
  // display login form 
  echo "&lt;h3&gt;Enter the admin password to add a photo&lt;/h3&gt;"; 
  echo "&lt;form action="photoalbum.php" method="post"&gt;"; 
  echo "Password:"; 
  echo "&lt;input type="password" name="password"&gt;"; 
  echo "&lt;input type="submit" value="login"&gt;"; 
  echo "&lt;/form&gt;"; 
} 
echo "&lt;/td&gt;&lt;/tr&gt;"; 


?&gt; 
&lt;/table&gt; 
&lt;/body&gt; 
&lt;/html&gt; 
?&gt;

Posted: Tue May 11, 2004 6:47 am
by patrikG
Edited the title to lowercase. It's very easy, connectsunny: just don't use CAPS LOCK and no-one will feel shouted at.

Oh and as magicrobotmonkey has posted: use

Code: Select all

around your code. Makes it much(!) easier.

Lastly: SQL Injection is something you will need to consider - your scripts are not secure at all. Have a look at this article ( http://www.sitepoint.com/article/794 ) and, if you need further info check http://www.google.com/search?q=sql+inje ... p;oe=utf-8.

thanks

Posted: Tue May 11, 2004 8:06 am
by connectsunny
I thought some1 would point out in the code itself ..... where i am wrong ! Can some1 make life easier for me coz i dont have much tym left for this assessment

Re: thanks

Posted: Tue May 11, 2004 8:09 am
by patrikG
connectsunny wrote:I thought some1 would point out in the code itself ..... where i am wrong ! Can some1 make life easier for me coz i dont have much tym left for this assessment
There is no point where you are "wrong". I pointed you towards one vulnerability - and I certainly won't do your homework for you :lol: :lol: :lol:

Re: thanks

Posted: Tue May 11, 2004 8:11 am
by JayBird
connectsunny wrote:I thought some1 would point out in the code itself ..... where i am wrong ! Can some1 make life easier for me coz i dont have much tym left for this assessment
Like a lot of us, our time is precious, but we help others in the hope of someday, recieving help ourselves.

Now, you haven't even bothered giving any background to the project, what the script is intended to do or what security issues you may be worried about.

If you think people have time to figure all this out themselves, without any help from the person wanting help (i.e. you) then you are not going to be succesful in your quest for answers.

Mark

Posted: Tue May 11, 2004 9:46 am
by mrvanjohnson
Validation of user input is the big one here. Refrence the links patrikG mentioned. I can't stress it enough validate validate validate.

Also, I think your authenication scheme is pretty weak. Your just doing a POST of a password (unencrypted)?

I also don't see where you are defining the type of files that can be uploaded. So people can upload Perl scripts or bad PHP scripts at execute them. Again validate all user input..

Posted: Tue May 11, 2004 11:12 am
by Weirdan
There is security hole if you're going to deploy this script to host with register_globals on. For such a hosts you should always initialize your variables with meaningful default values. With your script deployed on the host with register_globals on I would go to:
http://yourhost.com/yourscript.php?isAdmin=1
- and voila ;)

All the previous posters were right, your script is totally vulnerable to SQL injection.