Cubyz icon indicating copy to clipboard operation
Cubyz copied to clipboard

[WE] World edit `/blueprint` sub commands allow arbitrary paths

Open Argmaster opened this issue 10 months ago • 2 comments

Currently implementation of world edit /blueprint sub commands which take path argument perform no sanitization of the path itself, therefore they allow arbitrary file access via use of .. or other system specific path expressions. To alleviate this issues we can either refuse to accept paths containing specific sequences, constrain paths to predefined set of characters and or both.

In particular following sequences should not be allowed:

  • ..
  • ~
  • leading /

Argmaster avatar Apr 02 '25 05:04 Argmaster

Hmm right. At least you can only access .blp files through these commands, otherwise it would be a security issue.

To be on the safe side I would suggest to eliminate all characters that aren't allowed in file names. Otherwise the chances are big that we miss some niche pattern. e.g. what if you use an absolute path, or what if the path contains environment variables? Do those get resolved too?

IntegratedQuantum avatar Apr 02 '25 15:04 IntegratedQuantum

Generally it is up to the OS how it interprets paths from syscalls, but I don't think any OS does shell eval on them, so environment variables will not be resolved. Also <, > and | might be overkill for same reason. As for absolute paths, this most likely depends on zig's implementation of opening files, how it concatenates fileName when we give it in blueprintsDir.write(fileName, storedBlueprint). There are also control characters which technically can be injected via modified client, eg. null.

Argmaster avatar Apr 02 '25 15:04 Argmaster