fix: strip non-XML output from feed response to prevent script tag injection#147
Open
ahuininga-orisha wants to merge 1 commit into
Open
fix: strip non-XML output from feed response to prevent script tag injection#147ahuininga-orisha wants to merge 1 commit into
ahuininga-orisha wants to merge 1 commit into
Conversation
4f7eaa6 to
df4bd62
Compare
This file contains hidden or 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
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.
When third-party modules (such as the Stape server-side GTM module) attempt to set cookies or modify headers during a feed request, their output gets buffered in Magento's output buffer stack. When that buffer is eventually flushed, the garbage content (e.g. a
<script>redirect tag) ends up prepended to the XML feed. Tweakwise then rejects the feed during import because the XML is no longer valid.The fix clears all active output buffers immediately before streaming the feed, discarding any garbage already written by other modules, and then sets the
Content-Typeheader explicitly.FeedContentwrites directly tophp://outputbypassing PHP's OB layer, so the XML stream itself is unaffected — only the pre-feed buffered garbage is discarded.Changes
Controller/Feed/Export.phpob_end_clean()and setsContent-Type: application/xmlexplicitly beforeFeedContentstreams the feedHow to test
Scenario 1 — Normal feed renders correctly without third-party interference
php bin/magento cache:cleanhttps://{domain}/tweakwise/feed/export?key={feed_key}<?xml version="1.0" encoding="UTF-8"?>and ends with</tweakwise><script>tags are present anywhere in the responseScenario 2 — Feed renders correctly when a module outputs garbage before XML
echo "<script>alert(1)</script>";at the top ofController/Feed/Export.php::execute()before theob_end_clean()loop to simulate a rogue module<?xml— the injected script tag must not appearScenario 3 — Feed is valid XML and can be parsed
curl -o /tmp/feed.xml "https://{domain}/tweakwise/feed/export?key={feed_key}"xmllint --noout /tmp/feed.xml— must exit with code 0 and no errors