1

Here's my code:

<?php
if(isset($_GET['p']))
{
    $nshortname = strip_tags($_GET['p']);
    $check = mysql_query("SELECT * FROM pages WHERE `shortname` = '$nshortname'");

    if(mysql_num_rows($check) == 0) 
    {
        echo '<center><font size="50" style="font-weight:bold;">404</font><br>Appears this page is a dead end</center>';
    }
    else
    {
        $h = mysql_fetch_array($check);
        //post details
        $title = $h["title"];
        $content = $h["content"];
        $shortname = $h["shortname"];
        // Start of page content
        echo '

        <p>
        <font size="5pt">'.$title.'</font><br><hr>
        '.$content.'<br>
        ';
        // End of page content
    }
} 
else
{
    echo 'No page has been selected to view';
}
?>

What it does exactly, is it grabs pages from my database and reads them, so for example if I have a page in that table called "test" I can go to it by http://mylink.com/?p=test. Although i've come up with an issue. On one of those pages that come from the database I want to include but when I type it into the database field and go back to the page it shows with nothing.

I went to the source of the page in my browser and found out the code turned into <!--?php include "inc/extra/plugins/header/slideshow.php"?-->

Does anyone know how I can sold it from turning into <!--? and make my include code work.

Josh Crozier
  • 233,099
  • 56
  • 391
  • 304
  • you can use eval to Evaluate php code http://php.net/manual/en/function.eval.php – Breezer Nov 03 '13 at 08:41
  • @Breezer: Why do you think the OP needs `eval()` here? – Amal Murali Nov 03 '13 at 08:43
  • @AmalMurali as i understood it he retrieves "text" from the db that he then outputs, and one of thoose pages contain an include that doesn't get evaluated. I might have missunderstood the question though that's why i commented instead of answering – Breezer Nov 03 '13 at 08:45
  • @Breezer should my content field in my database contain: "eval("");" or where should I use eval in my codes? – user2594383 Nov 03 '13 at 08:47
  • you use evak on the $content variable, to get any php code inside the content to get run – Breezer Nov 03 '13 at 08:48
  • I read that its a dangerous command when used with input. I plan in the future adding comments to it, will it be dangerous or not? – user2594383 Nov 03 '13 at 08:49
  • @user2594383: Don't use `eval()`. It's not necessary here. And, what does `var_dump($content);` output? – Amal Murali Nov 03 '13 at 08:51
  • @AmalMurali "var_dump($output);" ? – user2594383 Nov 03 '13 at 08:53
  • @user2594383: Add `var_dump($content);` in your code (after `$shortname = $h["shortname"];`) – Amal Murali Nov 03 '13 at 08:54
  • @AmalMurali Added it and it says "string(56) " – user2594383 Nov 03 '13 at 08:55
  • So... You should use something to escape that user input before querying your database with it. Look up SQL injection. – JAL Nov 03 '13 at 10:54
  • @JAL do you mean the comments I said about? If so its all fine, its been checked for SQL injection. Although I still need this problem fixed... – user2594383 Nov 03 '13 at 11:06
  • That's definitely vulnerable code - strip_tags is not even relevant to SQL injection and you're not using mysql_real_escape_string, or mysqli/PDO. – JAL Nov 03 '13 at 11:17
  • I'd like to help with your issue, but feel like we need more info. It's somewhat confusing to understand. – JAL Nov 03 '13 at 11:20
  • @JAL I get what you mean, thanks for showing me that. And I dont see why its confusing? I have a code that runs ALL my pages from the database [meaning my content is stored in the database], but ive come accross a page where I NEED to have a PHP include in it. But I can not? – user2594383 Nov 03 '13 at 11:26
  • It's unclear where or how the – JAL Nov 03 '13 at 11:37
  • Im actually trying and find out how its converting to – user2594383 Nov 03 '13 at 11:40

2 Answers2

3

I would caution against using eval() of unknown content. Basically, the content comes from your database, but that doesn't guarantee it's safe to execute as code! There are a lot of ways it could cause errors or do something malicious.

But you also have other dangerous security gaffes in your code. You should learn about how to defend against SQL injection vulnerabilities and Cross-Site Scripting (XSS) vulnerabilities and File Inclusion vulnerabilities.

  • Use mysql_real_escape_string() if you are still using the deprecated ext/mysql. But if you can, switch to mysqli or PDO_mysql and use prepared statements with parameters.

  • Always output dynamic content with htmlspecialchars(). What if the content contains Javascript code? It could cause mischief.

  • Never eval() arbitrary content as code. You have no control over what that content is, or what it could do when you execute it.

  • Be as restrictive as possible - if you want to include a file, store the filename separately from content (e.g. in a separate column), and use it only for including files.

Here's an example with some of these problems fixed in your code:

<?php

if(isset($_GET['p']))
{
    $nshortname = mysql_real_escape_string($_GET['p']);
    $check = mysql_query("SELECT * FROM pages WHERE `shortname` = '$nshortname'");

    if(mysql_num_rows($check) == 0) 
    {
        echo '<center><font size="50" style="font-weight:bold;">404</font><br>Appears this page is a dead end</center>';
    }
    else
    {
        $h = mysql_fetch_array($check);
        //post details
        $title = htmlspecialchars($h["title"]);
        $content = htmlspecialchars($h["content"]);
        $shortname = $h["shortname"];
        // Start of page content
        echo '

        <p>
        <font size="5pt">'.$title.'</font><br><hr>
        '.$content.'<br>
        ';
        // End of page content

        // Start of include
        if ($h["include"]) {
          // strip out anything like "../../.." etc. 
          // to make sure this is only a simple filename.
          $include = basename($h["include"]);
          include "inc/extra/plugins/header/{$include}.php";
        }
        // End of plugin inclusion
    }
} 
else
{
    echo 'No page has been selected to view';
}
?>

Also check out http://www.sitepoint.com/php-security-blunders/ and http://phpsec.org/projects/phpsecinfo/


Re your comments:

To allow a limited set of basic HTML, the best tool you need to use is http://htmlpurifier.org

I'm not sure what to say about your include displaying code instead of working. I just tested this, and the following two files seem to work exactly as intended:

foo.php:

<?php

echo "<h1>START FOO</h2>";
if ($_GET["include"]) {
    $include = basename($_GET["include"]);
    include "./{$include}.php";
}
echo "<h1>END FOO</h2>";

bar.php:

<?php
echo "<h2>BAR</h2>";
Community
  • 1
  • 1
Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • Hi thanks for the security, anyhow.. with your code, it disallows all html. My pages that are read from the database require simple html such as , and – user2594383 Nov 03 '13 at 12:48
  • Wait dont worry about the basic html, but I still need to find out how to use correctly – user2594383 Nov 03 '13 at 12:55
0

If you have a variable $content which is html with php, you can use
eval("?>" . $content . "<?php");
This will output $content having processed all the <?php ?> tags.

Serge Seredenko
  • 3,541
  • 7
  • 21
  • 38
  • Hi, where would I add this? I've been trying to add it, although it just comes with a blank page. – user2594383 Nov 03 '13 at 11:34
  • Hi, I managed to solve this. Thanks! Was doing the wrong PHP code. By using 'Eval' will it be safe? I've heard it can be quite dangerous to use.. – user2594383 Nov 03 '13 at 11:58
  • @user2594383, No -- eval() of arbitrary content is *not* safe. What if I store content of `?`? Nor is interpolating $_GET variables into your SQL expression without escaping, nor is echoing content from your database without calling htmlspecialchars(). – Bill Karwin Nov 03 '13 at 12:03
  • @BillKarwin well, do you have any ideas in which could fix my problem, without using eval?? – user2594383 Nov 03 '13 at 12:08
  • @user2594383 Eval is ok. If you write code with your own hands, put it into the database and then eval, nothing will happen. Like others said, do not eval anything provided by users. But you are not going to, are you? – Serge Seredenko Nov 03 '13 at 13:11
  • Ah the main part = no. But in the script there will be a comment box [which would also be included..] so uhm.. – user2594383 Nov 03 '13 at 13:16
  • @user2594383 I don't get it. Do you need to allow users to run their php code? If yes, then you won't get rid of eval. If no, why do they need to write comments? – Serge Seredenko Nov 03 '13 at 13:24
  • @SergeSeredenko Sorry I dont understand your comment. I am having a blog type, thing where users can post their own comments. Would this effect me having eval? – user2594383 Nov 03 '13 at 14:27
  • @user2594383 And you want users to write php code in their comemnts? – Serge Seredenko Nov 03 '13 at 20:52
  • @SergeSeredenko No I dont, their comments will most likely be ran off using TinyMCE. – user2594383 Nov 04 '13 at 05:30