Welcome to the ZDoom Forums!

I have different installations of ZDoom-derived source ports, and I set them up to save logs into one folder with my main installation for the given port, so whenever I need to find something in them, I wouldn't have to remember the exact port I played back then, and could just search in one place. So... leave it be.

Chris wrote:It would also need to disallow '..' in the given path, which could break out of the log directory.

In software I write where this matters, I usually handle this by writing a path parser, which has some sort of "level" integer. Each directory increases it, while ".." decreases it and "." does nothing. You basically just deny paths that ever obtain a negative value.
So: "./some/folder" would be 2, "./some/../folder" would be 1 and "../some/folder" would also be 1, but because at one point the value is negative (-1, the "..") the path would get rejected.

AFADoomer wrote:I'm pretty sure that the main point of the 3.2.4 release was that the attack vector discusssed there was removed.

It's still an avenue for damage if some other exploit is found. It's also good to ensure functions are focused on doing what they were made to do, and if log files can be put into their own specific directory (like screenshots and savegames), it's good to ask if it's still worth the risk of letting the logfile command write everywhere the process is allowed to.

ZippeyKeys12 wrote:Ummm...
I think that's why AFADoomer.

I'm pretty sure that the main point of the 3.2.4 release was that the attack vector discusssed there was removed. This was done by adding a whitelist mechanism to the MENUDEF DoCommand implementation.

The logfile command is not on the whitelist for DoCommand.

Ummm...
I think that's why AFADoomer.

So, dumb question, but what would all of this work actually be intended to keep people from doing that they can't already do while, you know, using their computer?

I mean, yes a player could overwrite something important with a logfile or savegame, but they could just as easily go and delete that same file from their OS and cause the same problem...

Yeah, maybe having logfile check for the presence of any '/' or '\\' character and fail if either are present.

It would also need to disallow '..' in the given path, which could break out of the log directory. Probably symlinks too (though the user would have to purposely create them in the log directory targeting outside of it, so I dunno). Maybe it would be best to simply not allow paths in the logfile name at all.

That could work.

Rachael wrote:Ultimately the implementation of something like this has to be platform-specific. For obvious reasons you cannot restrict logfile naming on Linux platforms to the same folder as gzdoom - it has to go to /var/log/gzdoom or something like that, if the program is installed using root credentials.

What about saving log files in the same place as save games and the like? On Linux, this is ~/.config/gzdoom. Screenshots are placed in ~/.config/gzdoom/screenshots, so perhaps log files can go in ~/.config/gzdoom/logs (and the name given to the logfile command would be relative to this, an absolute path would be ignored).

Ultimately the implementation of something like this has to be platform-specific. For obvious reasons you cannot restrict logfile naming on Linux platforms to the same folder as gzdoom - it has to go to /var/log/gzdoom or something like that, if the program is installed using root credentials.

Then it gets even more complicated - you'll have to configure the package manager to ensure that the application has write access to such a folder.

I have no idea how Apple expects such a thing to be done. My guess is they probably have a system log system that works similarly to Windows' Event Viewer.

However, this is a really good idea and the implementation details do need to be ironed out, in my opinion, so that we can go forward with this.

Additional food for thought: instead of restricting the paths that can be used, require the file to match certain criteria.

An example could be something like: If the file doesn't exist, the path can be used. Otherwise, it must be empty or begin with something like Alternatively the logfile paths could still be restricted, but the user can specify additional "safe" paths in their .ini file (similar to the [IWADSearch.Directories] and [FileSearch.Directories] sections).

AFADoomer wrote:But if you're going to be thorough, the save command can take full paths as well, so is similarly problematic.

Though that brings to mind such use cases as running GZDoom off an external drive or on a system that does not necessarily have full write access (such as playing off a thumb drive on a public computer). Those would need to be accounted for, too, I would think.

I think the whole idea of artificially limiting save locations for something that has to be typed in by the player is overkill...

But if you're going to be thorough, the save command can take full paths as well, so is similarly problematic.

Kinsie wrote:[*]Add a prefix or suffix to the specified name, so as to avoid possibly overwriting any pre-existing files.

I'd suggest that said prefix be in a similar format to screenshots, i.e. [logname]-[date]-[start time of log].txt.

[edit] Furthermore, replace [logname] with "gzdoom" if the user just types in "logfile" with no arguments and there's not already a log going.