File/Rolling File appender doesn't guard against symlink attacks

[whiskeyjay]

The file/rolling appender should check for symlinks (or provide an option to turn the checking on/off) before writing or removing a log file.

A simplified repro on Windows (should be doable similarly on Linux):

1. Create a symbolic link using PowerShell command:
   New-Item -Path C:\logs\intended_log_file.log -Target C:\test\Important_sys_file.sys -ItemType SymbolicLink
   Before running the command, make sure 1) C:\test\Important_sys_file.sys actually exists, you can just put an empty file there; 2) C:\logs\intended_log_file.log doesn't exist.
2. Now run your app that uses log4rs with file appender that writes logs to C:\logs\intended_log_file.log.

You'll see the logs are actually being written to C:\test\Important_sys_file.sys. With rolling appender, files can be deleted by setting up symlinks.

If the Rust app runs with higher privilege than the attacker does, then it could be tricked into overwriting/deleting files the attacker normally wouldn't be able to get to.

I'm proposing to use the symlink_metadata function to check on the given log file path and make sure it is not a symlink to something else before writing or deleting. Would that make sense?

[bconn98]

I'm taking a look at this. Give me a couple days to consider side effects, but I don't think I see an issue.

[bconn98]

Some deeper consideration, symlink attacks are most often used as phishing schemes or something similar where the attacker owns both the executable and the configuration and the user is running with elevated permissions. Some information for those who find this thread in years to come more info here. Although, I do see the attack vector where the configuration is file based and an attacker replaces the config of a root level process with something malicious.

I tried digging into Log4j and Logback to see what their comments/feedback is on the issue, but haven't found much.

I guess my worry is a breaking change where someone might expect this behavior.

[whiskeyjay]

Thanks for looking into it.

My understand of this method of exploitation is that the attacker doesn't necessarily need to own the executable, as long as the targeted executable has the privilege of writing to the intended target file.

I do believe in general app developers should do due diligence to eliminate factors needed to mount a successful symlink attack, like not making the app run as elevated just because it's easier. And hence there shouldn't be a default behavior change in the log4rs lib. Besides, some people might even leverage the symlink feature to redirect the log files.

However, in case of an app that must run elevated, as its developer, I would wonder "how do I know when I write logs to xyz.log, I'm really writing to xyz.log instead of being tricked to write to something else?". It would be nice to have a mechanism to opt-in for a symlink check if the calling code chooses/needs to.