1

TL;DR

I'm using a PHP form to output directly to a PHP file. I realized it's a big security risk. Please tell me how to filter out any tags from the user's input, and limit the actions of the PHP file to its own directory (so as to minimize the damage in case of a compromise).

DETAILS

I have a page where I can collect feedback from customers using a PHP form, then output the results onto another PHP page where members of my team can read them.

<?php

if($_POST['CUSTFEEDBACK']) {
$addition = '<div class="FDBKbox">' . $_POST['CUSTFEEDBACK'] . '</div>';
$file_open = fopen("FDBKCOLLECTION.PHP", "a+"); 
fwrite($file_open, $addition);
fclose($file_open);
}

?>

<form action="<?=$PHP_SELF?>" method="POST">

<textarea name="CUSTFEEDBACK" style="width:100%;max-height:250px;">
</textarea>

<br>
<input type="submit" name="button" value="Submit Feedback">

</form>

Then it occurred to me that someone with access to both of these pages can also put in malicious code, open and rewrite other files from my server simply by adding PHP code, or execute XSS codes using my form.

Since then, I've changed the form action to <form action="<?=$PHP_SELF?>" method="POST"> to prevent XSS codes. However, other methods still pose a great risk for me.

At the moment, I'm using Javascript to replace any < or > in the textarea with a space. I also put a full-page div on top of the page with a script that gives it a "display:none" property so that the page will be unusable if Javascript is disabled. However, I soon found out that using an old browser (IE 8 and below), the browser simply ignored the script to replace tags. Using Javascript for site security was probably a bad idea to begin with, but I'll have to make do with it for now.

$(document).on('change',':text,textarea', function () {
    if (this.value.match(/[<>]/g)) {
        this.value = this.value.replace(/[<>]/g, '');
    }
});

I did some googling, and found some answers with methods like "escape" the tags, or using PHP to sanitize the form. However, I'm still relatively new to PHP and these all sound pretty intimidating at the moment (and I have no idea where to put those codes in).

Please tell me how I can remove any kind of tags from the form input (I'm not interested in any kind of markup in the textarea).

As a failsafe, how can I limit the actions of a PHP file to its own directory. This is so that in case of an unfortunate event, the damage could be minimized and all my other files can be protected.

If anyone could give me some pointers, that'll be greatly appreciated!


Side question, I'm currently studying PHP on codeacademy.com does anyone have suggestions for other good websites to learn PHP?

Panpaper
  • 451
  • 1
  • 6
  • 16
  • 1
    Possible duplicate of [open\_basedir - how to set for specific directory](http://stackoverflow.com/questions/13291185/open-basedir-how-to-set-for-specific-directory) – l'L'l Oct 01 '15 at 05:42
  • Thanks, but it only answers part of my question. – Panpaper Oct 01 '15 at 05:47
  • The other part of your question is off-topic (see [#4](http://stackoverflow.com/help/on-topic)), and not allowed by the rules. – l'L'l Oct 01 '15 at 05:49
  • Sorry, I was referring to "Please tell me how I can remove any kind of tags from the form input (I'm not interested in any kind of markup in the textarea)." – Panpaper Oct 01 '15 at 05:50
  • You need to show what you've tried, and what about it is not working. – l'L'l Oct 01 '15 at 05:51
  • Yes, I have. This entire paragraph is just that. "At the moment, I'm using Javascript [...] simply ignored the script to replace tags. " – Panpaper Oct 01 '15 at 05:52
  • 2
    @Panpaper Using Javascript to prevent code injection is a terrible idea. Users have 100% full control over the Javascript that runs on their machines. Do it server side. A simple Google search will show you how: http://php.net/manual/en/function.strip-tags.php – NStorm Oct 01 '15 at 05:54
  • I'm not seeing any `javascript` in your example, so I would recommend including it. – l'L'l Oct 01 '15 at 05:55
  • Thank you @NStorm, I didn't know what functions were available and what their names were. This would greatly help me get better search results. – Panpaper Oct 01 '15 at 05:58
  • @I'L'I Thanks, I'll include it now. – Panpaper Oct 01 '15 at 05:59

1 Answers1

2

There is no silverbullet solution for this. You will need to mix and match. Some points that might be helpful:

  • As suggested use open_basedir This limits the files that can be accessed by PHP to specified directory tree

  • You might not want to use strip_tags because it strips out all PHP tags and this behavior cannot be overridden. You might be better off regexing the tags - only thing to watch out for is that you do not strip PHP open/close tags along with it.

Edit: As pointed out in your comment, if you are creating a single monolithic PHP file, then you can strip_tags and then add PHP tags to the beginning and end.

  • Once you have this stripped out version, you want to make sure that it is still a valid PHP code. You can exec php -l <filename> and examine the output to see if syntax errors are present Trivia: Between versions 5.0.1 to 5.0.5, PHP had a function called php_check_syntax using which you could check syntax of one PHP file from another.

Finally it is not recommended at all to have such a system anywhere in production. For testing/academics/research etc its fine. In production - not so fine.

raidenace
  • 12,789
  • 1
  • 32
  • 35
  • Thank you so much, this is very helpful and thorough! From what I read, strip_tag removes the tags from a string, why would it not be appropriate? I would have the stripped content written to a PHP file, then include this PHP file from another page. And I will definitely change this system in the future when I get better at it, thanks again! – Panpaper Oct 01 '15 at 06:43
  • This would be inappropriate in cases where you have multiple PHP tag opened and close (spaghetti code), or if there are heredocs. However when I think about it, you are correct that this case is unlikely since you already mentioned that you are not concerned about HTML tags to begin with. You might also want to check if strip_tags handles self-closing tags like
    .If it does, then yeah I guess it would work :)
    – raidenace Oct 01 '15 at 06:48
  • I thought about that, my current solution is to have a styled
     tag before and after the `php include`. This way, the input wouldn't need any 
    . I'll go ahead and accept your answer because I found it most informative. Gracias!
    – Panpaper Oct 01 '15 at 07:00