search

pencil2d / pencil

Pencil2D is an easy, intuitive tool to make 2D hand-drawn animations. Pencil2D is open source and cross-platform.
http://pencil2d.org
GNU General Public License v2.0
1.45k stars 272 forks source link

Prevent Pencil2D projects from referencing external files #1843

Closed scribblemaniac closed 3 months ago

scribblemaniac commented 4 months ago

This PR adds additional checks to the loading of bitmap/vector/sound layers so that they will not be loaded if they are outside of the data directory. This can occur with specially crafted main.xml files, or with symlinks. This closes multiple admittedly unlikely security threats in the program that can result in exfiltrating images from arbitrary locations though animations, and even duplicating arbitrary files under some circumstances. Tests have also been written for the scenarios this PR covers with the exception of symlinks (and possibly Windows shortcuts) because Qt has very bad support for symlink creation.

This also incidentally fixes an issue where an infinite loop will occur when loading a main.xml file that contains a non-element node (ex. an xml comment) in the sound layer.

scribblemaniac commented 4 months ago

I was about to approve this as it looks good to me now but i noticed that you haven't made this change for the camera layer, is that intentional?

Yes this is intentional. As far as I know, the camera layer doesn't read any data from other files (all the camera data is the in the main.xml file) so it does not need these checks.

scribblemaniac commented 3 months ago

Thanks for the reviews @MrStevns and @chchwy

chchwy commented 3 months ago

All good. BTW, please use "Squash and Merge" next time. It gives a proper granularity of commits, making it clearer when reviewing the git history.