Skip to content
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

Fix for: https://mantis.ilias.de/view.php?id=31016 #4016

Conversation

kergomard
Copy link
Contributor

This PR changes the way images are converted, but tries to keep as much of the sanitation provided by escapeshellcmd as possible to avoid any potential vulnerabilities. As discussed in the meeting of the Taskforce on Bugfixing it moves most of the logic to execute convert to the Test.

I decided to go with this summary approach instead of calling escapeshellarg() on every arg. Technically this would probably be cleaner, but it would mean a gazillion calls to escapeshellarg().

I would personally also like to rename the variable "convert_cmd" to "convert_args" as I think this would make its purpose clearer as there is no command to be found nowhere in the assigned string. I can expand this PR accordingly, if you want.

I can also provide a PR for Trunk, if you want, as Fabian has just cleaned up the mess called ilUtil and thus this can't simply be cherry-picked.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
This commit changes the way images are converted, but tries to keep as
much of the sanitation provided by escapeshellcmd as possible to avoid
any potential vulnerabilities.
@kergomard kergomard added bugfix php Pull requests that update Php code labels Feb 9, 2022
@nicoroeser
Copy link
Contributor

Related to #3873.

@mbecker-databay
Copy link
Contributor

Thank you so much for this fix!

@mbecker-databay mbecker-databay merged commit 8def3d1 into ILIAS-eLearning:release_7 Feb 23, 2022
mbecker-databay added a commit that referenced this pull request Feb 23, 2022
Fix for: https://mantis.ilias.de/view.php?id=31016
# Conflicts:
#	Modules/TestQuestionPool/classes/class.ilImagemapPreview.php
@kergomard kergomard deleted the fix_imagemap_question branch September 27, 2022 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix php Pull requests that update Php code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants