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

Rel 702 fixups #7485

Merged
merged 6 commits into from
Jun 26, 2024
Merged

Rel 702 fixups #7485

merged 6 commits into from
Jun 26, 2024

Conversation

stephenwaite
Copy link
Sponsor Member

Fixes #

Short description of what this resolves:

Changes proposed in this pull request:

@@ -109,6 +109,7 @@ public function doRender($template_id, $template_content = null, $json_data = nu
// purify html (and remove js)
$isLegacy = stripos($template, 'portal_version') === false;
$config = HTMLPurifier_Config::createDefault();
$config->set('Cache.SerializerPath', $GLOBALS['temporary_files_dir']);
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a subfolder in the temp directory for this? Probably overly paranoid but just wondering if there's a risk of other files in the apache php processor space being overwritten (session files, upload files). It'd be a supply chain attack on htmlpurifier, so probably too paranoid here.

Copy link
Sponsor Member Author

@stephenwaite stephenwaite Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondered about that but we need a writable folder somewhere

commit in master

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opened #7520 to address this but maybe you meant a separate dir in /tmp not in sites/default/documents/temp then the system purges it instead of having it hang around in documents

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was meaning a separate dir in /tmp. Depends on how much we care about shared hosts. Share hosts sometimes lock down the /tmp directory and writing to the configured OpenEMR temp directory would probably be the least surprising for someone whose configured that global option. Not sure if HTMLPurifier cleans up its own cache files though.

…#7521)

* fix: create custom temp directory for html purify serializer

* make subdir in temporary_files_dir

* use constant instead of slash

* log mkdir error
@stephenwaite stephenwaite merged commit ef74254 into openemr:rel-702 Jun 26, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants