Skip to content

WIP: Xxe - #780

Merged
MarkBaker merged 9 commits into
developfrom
xxe
Nov 20, 2018
Merged

MarkBaker merged 9 commits into
developfrom
xxe

Conversation

@MarkBaker

@MarkBaker MarkBaker commented Nov 20, 2018 •

Copy link
Copy Markdown
Member

This is:

- [ ] a bugfix
- [X] a security fix
- [ ] a new feature

Checklist:

Why this change is needed?

Changes to the xml security scanner to use libxml_disable_entity_loader() when cleanly supported and thread-safe, and to handle UTF-7 charset which otherwise permits an XXE exploit

Comment thread src/PhpSpreadsheet/Reader/Security/XmlScanner.php Outdated
@MarkBaker

MarkBaker commented Nov 20, 2018 •

Copy link
Copy Markdown
Member Author

Need to enforce "doctrine/instantiator": "1.0.5" in composer.json (or ^1.0.0) for PHP 5.6; doctrine/instantiator 1.1.0 (if allowed) will break unit tests against PHP 5.6... composer should handle ^1.0.0 correctly when running on travis against 5.6

@PowerKiKi PowerKiKi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should probably use lowercase method name

Comment thread src/PhpSpreadsheet/Reader/Security/XmlScanner.php Outdated
Comment thread src/PhpSpreadsheet/Reader/Security/XmlScanner.php Outdated
PowerKiKi and others added 2 commits November 20, 2018 11:46
Co-Authored-By: MarkBaker <mark@lange.demon.co.uk>
Co-Authored-By: MarkBaker <mark@lange.demon.co.uk>
Comment thread src/PhpSpreadsheet/Reader/Security/XmlScanner.php Outdated
@MarkBaker
MarkBaker merged commit 0f8f071 into develop Nov 20, 2018
BlackyTay pushed a commit to BlackyTay/PhpSpreadsheet that referenced this pull request Aug 8, 2025
Changes to the xml security scanner to use libxml_disable_entity_loader() when cleanly supported and thread-safe, and to handle UTF-7 charset which otherwise permits an XXE exploit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants