Log In  

The new printh() does not sanitise its second argument (the filename) and can thus create or modify user-owned files anywhere on the filesystem. For instance the following code could be put in a cartridge and run on Linux:

printh("echo 'lol'", "../../../.bashrc")

It will cause the command to be executed the next time the user logs in (provided they use bash).

A similar technique works on Windows:

printh("@echo lol", "..\\..\\microsoft\\windows\\start menu\\programs\\startup\\script.bat")

Many variations of the above are possible and potentially devastating.

P#28553 2016-09-15 16:21 ( Edited 2016-09-19 02:29)

Good call @samhocevar.

Agreed that file operations should be limited strictly to specific data directories. I'd go as far as to say that printh should get its own Pico-8 subdirectory (maybe "logs" or something) and should not even be allowed to write into the same dir as carts, though that might have tradeoffs for novices. Subdirs in here are fine and would be useful to log different types of messages to different files for the same project.

I might also ask to limit the dimensions of the output file: maximum line length, maximum file size that rolls off the first n lines as needed. Args can override default limits, and maybe disable them.

A command line arg that disables printh might also be good to have.

P#28561 2016-09-15 19:08 ( Edited 2016-09-15 23:08)

Ouch, thanks I'll get a fix for this available asap in 0.1.9b

@dddaaannn perhaps forcing an extension ".p8l" would provide safety within the pico-8 file tree but allow the convenience of storing logs in the same folder as the carts that produced them.

Max file length is definitely needed to prevent disk-filling carts (cstore also has limits for this reason). Not sure about line length though -- Is that a safety thing, or just to avoid unwieldy files?

P#28564 2016-09-15 19:44 ( Edited 2016-09-15 23:44)

Perhaps save data in folders the same name as the project or active .P8 file of the time ?

P#28570 2016-09-15 21:56 ( Edited 2016-09-16 01:56)

Not sure if I have a complete idea around limiting line length, but it occurred to me while thinking about rolling off the top lines of a file, as opposed to the top bytes of a file. Worst case is appending to a gigantic file that doesn't contain a newline, forcing a scan of the entire file. Probably not an issue if the cart can't reasonably append to a file that it shouldn't and can't create such a file.

While we're here, there's also the possibility of creating a ridiculous number of files. It might be nice to limit a run of a cart to 100 unique filenames.

(I wish I had more filesystem security best practices at my fingertips beyond sanitizing user-provided paths. I'm always surprised by security write-ups.)

P#28621 2016-09-16 17:33 ( Edited 2016-09-16 21:33)

I think it's more the point that you can use ".." in the directory structure to navigate OUT of the PICO-8 filetrees; and on Linux, to the computer root itself. That should probably be removed entirely (and any future PICO-8 release's functionality to execute that), and that editable subdirs should have to match something in PICO's internal config... so that the only way a .p8 cartridge could edit your main filesystem is if YOU, the end-user, configure PICO specifically to do so. (Then it's on the user.) All other edits are limited to user documents and their roaming AppData. I wouldn't even include the Desktop; because it can have the same kind of implications there (like making an invisible shortcut).

It's probably also wise to not allow .p8 carts to internally edit the PICO-8 config files, to reopen that doorway, too.

P#28636 2016-09-16 21:46 ( Edited 2016-09-17 01:47)

!/bin/bash

sudo rm -rf /

and have printh put that somewhere with the name "readme" ... oh dear ...

P#28777 2016-09-18 22:29 ( Edited 2016-09-19 02:29)

[Please log in to post a comment]

Follow Lexaloffle:          
Generated 2024-03-29 07:19:56 | 0.015s | Q:23