Prevent Pencil2D projects from referencing external files #1843
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MrStevns
reviewed
May 29, 2024
MrStevns
reviewed
May 29, 2024
MrStevns
reviewed
May 30, 2024
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. |
chchwy
reviewed
Jun 4, 2024
scribblemaniac
force-pushed
the
external-ref-block
branch
3 times, most recently
from
9f6d71f
to
435aa63
Compare
chchwy
force-pushed
the
external-ref-block
branch
from
435aa63
to
3922d48
Compare
chchwy
approved these changes
Jun 6, 2024
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
scribblemaniac
force-pushed
the
external-ref-block
branch
from
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.