1

Recently I wrote this code:

<?php 
$path = '../data/about.html';
$handle = fopen($path, 'w') or die('Cannot open file:  '. $path);
$data = $_POST['name'];
fwrite($handle, $data);
fclose($handle);
header("Location: warnings.php")
?>

I heard that this code is vulnerable to XSS injections and null bytes. How can I make it more secure? Note that the files I wrote will later be used with AJAX on pages, so I need it a way I can decrypt in JS if I need to encrypt files.

Anders
  • 64,406
  • 24
  • 178
  • 215
MucaP
  • 113
  • 4

1 Answers1

5

This looks like something that needs to be redesigned entirely. The main issue is that you are allowing the contents of the about.html file to be written to via a web request. The user can submit a request will allow them to render arbitrary html.

Can the markup in about.html be templated? If so, then you can escape the fields for the appropriate context (attribute vs. element), but as it stands right now there is not a good way to secure this code.

Edit for clarification:
It sounds like you may need a better understanding of what Cross-Site Scripting (XSS) is. XSS is acutally a terrible name for the vulnerability. What is really occurring is client side code injection. More information is available here: https://www.owasp.org/index.php/Cross-site_Scripting_(XSS) The heart of an xss vulnerability is the fact that an attacker can control what the user's browser (render markup and execute arbitrary JavaScript).

The issue with the current implementation is that you are trying to let a user write an entire html document. With your solution, the user needs to be able to write some html, but should not be allowed to write malicious html. This is extremely difficult to do correctly. Escaping the html (using htmlspecialchars, in this instance) being a part of the correct solution, but I don't believe it it will work with the current implementation. A much easier solution would be to template out the about.html file, and escape all of the data that the user is passing in. This way none of the markup will be controlled by the user.

Dan Landberg
  • 3,312
  • 12
  • 17
  • Good answer! I think it would be even better if you included some info about how this could be exploited by an attacker. Like (1) send some JS (e.g. that reads cookies and send them off to `evil.com`) in the `name` parameter, and (2) fool the victim into visiting the `data/about.html` page. Bingo, you have just stolen the victims cookies. – Anders Feb 07 '17 at 08:27
  • Nice, this is the correct answer. I tried to use htmlspecialchars, but I think i did not use it properly. Is using this a solution, and how? – MucaP Feb 07 '17 at 12:41
  • Hi @MucaP, I've updated my answer to hopefully add some clarification. htmlspecialchars is part of the solution, but will not work with the current implementation. – Dan Landberg Feb 07 '17 at 15:25