6

Do I need to sanitize user input from a public facing form when passing data straight into:

error_log();

http://php.net/manual/en/function.error-log.php

I know this is a rather simplistic question but I can't find anything elsewhere.

Thanks

Jack
  • 3,271
  • 11
  • 48
  • 57
  • Sanitation I'd say no, but be careful with putting passwords/usernames in there, as they will be plainly visible to whomever can access the logfiles. – Bart Friederichs Aug 24 '15 at 19:32
  • 2
    `Tip: message should not contain null character. Note that message may be sent to file, mail, syslog, etc. Use appropriate conversion/escape function, base64_encode(), rawurlencode() or addslashes() before calling error_log(). ` That's what the manual says. – Charlotte Dunois Aug 24 '15 at 19:36
  • 1
    What @Bart said. Logs are more about information leaks than filtering for security. Your log viewer should just be more robust for displaying. (Just `cat`ing a log would expose control chars / colorization undesirably, and an online viewer must of course escaping stray HTML itself, etc.) – mario Aug 24 '15 at 19:37
  • `addslashes()` doesn't help here, it only clutters the output. – axiac Aug 24 '15 at 20:24
  • 1
    @CharlotteDunois has quoted from the [manual](http://php.net/manual/en/function.error-log.php). Assuming you don't pass the optional arguments, [`error_log()`](http://php.net/manual/en/function.error-log.php) puts the text it receives in the first argument into a file. You don't need extra slashes or other types of quoting or escaping in a text file. All you have to do is to make sure the text you log is human-readable and not some binary garbage. Anything else just makes the log difficult or impossible to read without using a decoding tool and this defeats the purpose of the log file. – axiac Aug 24 '15 at 20:32
  • 1
    With `tail` it would mainly be ANSI / terminal control sequences to worry about (clear screen / clear line / etc). Though it really depends on where your log data/strings originate from. Addslashes will not help, but `addcslashes($v, "\0..\037")` might do. – mario Aug 24 '15 at 20:32

4 Answers4

2

The OWASP page regarding log injection attacks is helpful: https://owasp.org/www-community/attacks/Log_Injection

It is possible for someone to inject data into your logs that you wouldn't want there, such as characters that could corrupt the file (depending on what you use to process/read the file later), or just data that wasn't actually logged (forged log lines).

If someone can inject false data into your logs, they can make it harder to audit the logs and determine what has happened in the case of a compromised server.

Adding some basic sanitization could save you potential headaches, particularly down the road if you change how you process/read your logs and forget about the code you are writing today.

Like any security, it is a layer of the onion. You have to look at everything that touches those logs and determine what level of risk is acceptable and how much effort is warranted to mitigate potential issues. How does your app use the logs, how do you process/view the logs, how do those logs get used on the server (eg: fail2ban uses logs to determine who to ban from connecting to the server in various ways)?

There is a good answer here from someone that prefers not to filter/sanitize the data put into their own logs but explains how they prevent issues: https://stackoverflow.com/a/55199264/2153218

They mention that they filter out new lines to ensure there is only ever one line logged at a time, and beyond that they ensure the ways they use/view the logs are not vulnerable.

Lucha Laura Hardie
  • 429
  • 1
  • 3
  • 10
1

You don't need to sanitize data, if you log Request data. Then for debug, you would want to know what was send on request.

It this is after validation, then still no, as you should have valid data.

The only problem what you need to resolve, to not log important data in security context.

timiTao
  • 1,417
  • 3
  • 20
  • 34
1

As the data is just passed to a file you don't have to worry about sanitising the data.

If you wanted to use that file elsewhere and display it on a website or parse it in some way you will need to sanitise the data. Basically, no :)

chalky888
  • 11
  • 2
1

If you want to play it safe:

error_log(addcslashes($message, "\000..\037\177..\377\\"));

that will encode all non-printable and non-ASCII characters (i.e. any byte that wouldn't match /[\x20-\x7E]/) and double any original backslash

user14762539
  • 26
  • 2