Can input written to a file be maliciously tampered?

Question

Uber simple example to illustrate the point:

$message = $_POST['message'];

$fp = fopen("log.txt", "a");
fwrite($fp, $message);

fclose($fp);

Should I be sanitizing user input for the $_POST['message'] variable?

I understand prepared statements (for database sanitization) and htmlentities (if I were outputting the POST message back to the screen at some time) but in this case, the input is simply sitting in a log file that will be read by a small PHP script (via fopen())

Is the answer dependent on how it will be read? For example if I do open the log file via fopen() it should be htmlentities, and if I plan to download the log file and read it with Excel (for filtering purposes), there is nothing to be done?

Answer 1

Your code is basically innocent. The only "obvious" attack would be to repeatedly upload data to your server, eventually exhausting your disk space.

"sanitizing" is something that's situational. It's not something you can just sprinkle on code to make it better, like you can with salt on food. Perhaps you'll sanitize the $_POST data to prevent SQL injection attacks, but then use the data in an HTML context - now you're vulnerable to XSS attacks. Perhaps it's an image upload, and you do basic MIME-type determination to make sure it IS an image. That's all fine and dandy, but then someone uploads kiddy porn, which will pass the "is it an image" test, and now you've got a much bigger problem.

Since you're accepting user data and writing it out to a file, there is nothing that can be done with this code (except the disk space problem) to abuse your system. You cannot embed some data sequence into the data that'd cause PHP, or the underlying OS, to suddenly stop writing that data out to disk and start executing it. It doesn't matter WHAT kind if data is being uploaded, because it's never being used in a context where it could be used to affect the script's execution. You're simply sucking in some data from the webserver, and spitting it out to disk. You're not allowing the user to influence which file is written to (unless your users have shell-level access to the server and could, say, create a symlink called 'log.txt' pointing at some OTHER more critical file).

The real problem comes AFTERWARD... what do you do with this file after it's been written? If your later code does something silly like

include('log.txt');

then now you DO have a problem - you've now taken this "innocent" data sitting in a file on the disk and turned it into potentially executable code. All it takes is a simple <?php exec('rm -rf /') ?> anywhere in that file to trash your server.

As well, consider something like the inherently idiotic "security" measure that was PHP's magic_quotes. The PHP developers (WRONGLY and STUPIDLY) assumed that ANY data submitted from the outside world would only EVER be used in an SQL context, and did SQL escaping on ALL data, regardless of its ultimate purpose. And to make it worse, they simply assumed that all databases use backslashes for their escape sequence. That's all fine and dandy if you never use anything but MySQL, but what if you're on, say, SQL Server? Now you have to translate the PHP-provided Miles O\'Brien to Miles O''Brien, essentially having to UNDO what PHP did for you automatically.

TL;DR: Don't use shotgun 'sanitization' methods, they're almost always useless/pointless and just involve more work before AND after. Just use context-specific methods at the time you're using the data.

Answer 2

You should sanitize user input, but how is entirely dependent on what the input is for. "Sanitizing" refers to the idea of making sure input is safe or sane for a particular use. The term cannot be more specific until you settle on use cases.

You don't need to worry about the PHP reading/writing functions like fopen(). Be concerned with steps that actually parse or analyze the input. Some possible examples:

• If a file will be displayed in a basic log reader, you might need to make sure that each input is limited to a certain length and doesn't contain line breaks or your chosen field delimiter, and the beginning of each line is a valid time stamp.
• If a file will be displayed in a web browser, you might need to make sure inputs do not include scripts or links to other resources (like an IMG tag).
• Excel files would have similar concerns regarding line length, time stamps, and delimiters. You don't have to worry about someone including executable code as long as Excel will be parsing the file as text. (Also, modern Excel versions give you warnings about included macros before running them.)