-
Notifications
You must be signed in to change notification settings - Fork 272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent Pencil2D projects from referencing external files #1843
Prevent Pencil2D projects from referencing external files #1843
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
9f6d71f
to
435aa63
Compare
435aa63
to
3922d48
Compare
The code for doing the data dir checks has been modified in two significant ways: - If the file does not exist, parent directories are still checked for symlinks - If the data directory of one of its parent directories is a symlink, this is allowed and should be handled correctly now
3922d48
to
a82bd7b
Compare
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. |
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.