Skip to main content.

Sunday, May 29, 2005

What's wrong with this code?

Here's a challenge: I have a piece of PHP code, and it's up to you to find the possible issues with it. Security issues, code issues as well as issues with different PHP configurations and versions. Look at the code from the Nucleus perspective: a PHP script which will be installed on many different systems (for Nucleus: PHP versions 4.0.6 and up), and should run out of the box.

Solutions can be found in a follow-up post.

About the script

It's a really lame script: it displays an input form, where you can enter your name. The script will then lookup a matching greeting from the database and display it. If no greeting can be found, it will display a simple "Welcome Again!".

There's one special case. If "It's karma" is entered in the form field, the script will simply display "Hi Karma!" and quit.

The script

1  <?
2    switch ($HTTP_POST_VARS['name'])
3    {
4      case "It's karma":
5        echo 'Hi Karma!';
6        exit;
7      case "":
8        showform();
9        exit;
10     default:
11       showgreeting();
12       showform();
13       exit;
14   }
15  
16   function showForm()
17   {
18     echo '<form method="post">';
19     echo '<input name="name">';
20     echo '<input type="submit">';
21   }
22
23   function showGreeting()
24   {
25     global $name;
26     
27     // get greeting from database
28     mysql_connect('localhost');
29     mysql_select_db('test');
30     
31     $query = "select greeting from test "
32     $query .= where name='" . $name . "'";
33
34     $r = mysql_query($query);
35     if (mysql_num_rows($r) > 0)
36     {
37       $o = mysql_fetch_object($r);
38       $greeting = $o->greeting;
39     } else {
40       $greeting = 'Welcome again!';
41     }
42    
43     mysql_close();
44    
45     // each <%user%> will be replaced by the user name
46     $template = 'Hi <%user%>! <%greeting%>';
47    
48     $text = str_replace('<%user%>', 
              $name, $template);
49     $text = str_replace('<%greeting%>',
              $greeting, $text);
50    
51    echo $text;
52   }
53
54 ?>

Posted by karma at 22:03:09. Filed under: PHP Tips

Comments

a quick browse caught a few:

- showform() should be showForm()
- same for showgreeting()
- line 32 missing ;

Do I won a price? 8)

Posted by admun at Monday, May 30, 2005 05:32:20

@1: Short tags may be disabled.

@2: As of PHP 5.0, the long predefined HTTP_POST_VARS variable can be disabled (register_long_arrays). So it is quite possible this variable does not exist.

@4: If magic_quotes_gpc is turned on this case statement will fail, because single quotes are escaped.

@16: A form tag is opened by this function, without the required closing tag.

@25: As of PHP 4.2 register global is turned off by default, so importing $name from the global scope will not work on a default install.

@25: The method of the form is 'POST'. The global that is imported here could be provided by the 'GET' method.

@28: The database connection is opened with a default mysql database username and password.

@32: Unchecked and unescaped user supplied variable is inserted into a query.

@48: Unchecked and unescaped user supplied variable is inserted into the output of the page.

@48,@49: Output should probably be entity-encoded.

Posted by rakaz at Monday, May 30, 2005 18:21:58

Add Comment

This item is closed, it's not possible to add new comments to it or to vote on it

Previous Item - Archives - Next Item