CVE-2026-75650 / APSB26-146: Magento built the object before it checked the class type - A look at Adobe’s VULN-39341 Patches
This post discusses the StyleSmuggler exploit, discovered by my good friends at Sansec.
Adobe has shipped a set of composer patches under the internal reference VULN-39341, one build for each supported Magento line: 2.4.4-p18, 2.4.5-p17, 2.4.6-p15, 2.4.7-p10, 2.4.8-p5 and 2.4.9. I diffed all six against each other first, because that usually tells you something. They are byte-for-byte the same fix. The only differences are hunk offsets, blob hashes and one line where the older branch still uses const instead of public const. Every supported version gets the identical eight-file change, which means the bug has been in the platform since at least 2.4.4 and probably a lot longer.
What follows is an analysis of what the patch closes, based on the patch itself and on reading the pre-patch code in a real 2.4.7 vendor tree. There is no working exploit here and I am not going to publish one. The interesting part is the shape of the bugs, because two of them are patterns you have almost certainly written yourself.
The eight files
vendor/magento/framework/View/Element/BlockFactory.php vendor/magento/framework/View/Layout/Generator/Block.php vendor/magento/framework/Webapi/ErrorProcessor.php vendor/magento/module-backend/Model/Widget/Grid/Row/UrlGeneratorFactory.php vendor/magento/module-email/Block/Adminhtml/Template/Preview.php vendor/magento/module-email/Model/AbstractTemplate.php vendor/magento/module-newsletter/Block/Adminhtml/Queue/Preview.php vendor/magento/module-newsletter/Block/Adminhtml/Template/Preview.php pub/errors/processor.php
Three distinct problems, and they are worth separating because they carry very different risk.
Problem one: instantiate first, type-check second
This is the important one. BlockFactory::createBlock() looked like this:
public function createBlock($blockName, array $arguments = []) { $blockName = ltrim($blockName, '\\'); $block = $this->objectManager->create($blockName, $arguments); if (!$block instanceof BlockInterface) { throw new \LogicException($blockName . ' does not implement BlockInterface'); } ...
Read the order of operations. The class is constructed, and only then is it asked whether it was ever allowed to exist. By the time the instanceof check runs and throws, the constructor has already executed. So has every constructor in the dependency graph the object manager resolved to build it, and so has any __destruct that fires when the rejected object is garbage collected.
That is arbitrary object instantiation, which in PHP is a well-worn route to worse things. You are not looking for a class that does something useful when you call a method on it, because you never get to call a method. You are looking for a class whose construction has a side effect: one that writes a file, opens a connection, deserializes something or hands a value to a sink further down. Magento’s object manager can construct a very large share of the classes in the codebase, plus everything in vendor/, and the arguments array is under the caller’s control too.
The fix is to resolve the DI preference for the requested name and type-check the string, before anything gets built:
$resolvedType = $this->objectManagerConfig->getInstanceType( $this->objectManagerConfig->getPreference($blockName) ); if (!is_a($resolvedType, BlockInterface::class, true)) { throw new \LogicException($blockName . ' does not implement BlockInterface'); } $block = $this->objectManager->create($blockName, $arguments);
is_a() with the third argument set to true accepts a class name string rather than an object, so the check happens with nothing instantiated. Resolving the preference first matters, because the name passed in is frequently an interface or a virtual type, and checking the raw name would reject legitimate blocks.
The same pattern, and the same fix, in UrlGeneratorFactory::createUrlGenerator(). Before:
$rowUrlGenerator = $this->_objectManager->create($generatorClassName, $arguments); if (false === $rowUrlGenerator instanceof GeneratorInterface) { throw new \InvalidArgumentException('Passed wrong parameters'); }
After, with Adobe’s own comment left in the patch:
// Validate the type BEFORE instantiation. if (!is_a($generatorClassName, GeneratorInterface::class, true)) { throw new \InvalidArgumentException('Passed wrong parameters'); } return $this->_objectManager->create($generatorClassName, $arguments);
That one is reached from Magento\Backend\Block\Widget\Grid\ColumnSet::__construct(), which reads the generator class name straight out of block data:
$generatorClassName = \Magento\Backend\Model\Widget\Grid\Row\UrlGenerator::class; if (isset($data['rowUrl'])) { $rowUrlParams = $data['rowUrl']; if (isset($rowUrlParams['generatorClass'])) { $generatorClassName = $rowUrlParams['generatorClass']; }
Anywhere block data is influenced by something less trustworthy than a developer editing layout XML, that is a class-name sink.
Why the block factory is the one that matters
A factory buried in the framework only matters if something can feed it a name. Something can.
Magento\Email\Model\Template\Filter::blockDirective() is what implements the {{block class="..."}} directive:
if (isset($blockParameters['class'])) { $block = $this->_layout->createBlock($blockParameters['class'], null, ['data' => $blockParameters]); }
Layout::createBlock() goes to Layout\Generator\Block::getBlockInstance(), which goes to BlockFactory::createBlock(). And Magento\Cms\Model\Template\Filter is a one-line subclass:
class Filter extends \Magento\Email\Model\Template\Filter
So the directive is live in CMS pages, CMS blocks and widget content, and the result renders on the storefront to anonymous visitors. Pre-patch, the class named in that directive was constructed before anyone asked whether it was a block. An admin whose role grants nothing but CMS editing could reach a construction primitive across most of the codebase, and see the fallout on a public page. That is a privilege escalation inside the admin, not just a hardening nit.
The email and newsletter template editors reach the same directive by the same route, which is why the preview blocks are in this patch at all.
There is a small companion change in Layout\Generator\Block that tells you the new check throws in places the old code never expected an exception:
- } catch (\ReflectionException $e) { + } catch (\ReflectionException | \LogicException $e) {
Without it, a layout naming a non-block class would surface the new LogicException as a fatal instead of a logged critical. Worth knowing if you have a theme or a module that names something odd in layout XML: post-patch it will log and render empty rather than blow up, but it also will not silently construct the object any more.
Problem two: request parameters typed as “whatever arrived”
Magento\Email\Block\Adminhtml\Template\Preview::_toHtml() builds an unsaved template out of raw request input:
$template->setTemplateType($request->getParam('type')); $template->setTemplateText($this->_maliciousCode->filter($request->getParam('text'))); $template->setTemplateStyles($request->getParam('styles'));
getParam() returns whatever the query string or POST body says, and PHP will happily give you an array from text[]=a&text[]=b. The filter in the middle does not stop that. Magento\Framework\Filter\Input\MaliciousCode::filter() is documented as taking string|array, and it runs preg_replace() over the value, which returns an array when handed one:
/** * @param string|array $value * @return string|array */ public function filter($value) { $replaced = 0; do { $value = preg_replace($this->_expressions, '', $value ?? '', -1, $replaced); } while ($replaced !== 0);
So an array, or a nested array, survived into setTemplateText(), which is plain magic-method setData() with no type at all, and from there into the template filter which expects a string throughout. The patch replaces the magic setters with real ones on Magento\Email\Model\AbstractTemplate:
public function setTemplateText($value) { return $this->setData('template_text', is_string($value) ? $value : ''); } public function setTemplateStyles($value) { return $this->setData('template_styles', is_string($value) ? $value : ''); }
Coerce to empty string rather than throw, so nothing that legitimately passes null keeps working. Note also that styles was never passed through the malicious-code filter at all, and still is not: the string coercion is the whole of its protection.
The lesson generalises past this patch. getParam() is not a string. Every setSomething($request->getParam(...)) in your own code is an untyped hand-off, and Magento’s magic setters will store an array as cheerfully as a string.
Problem three: attacker-controlled strings written to disk as PHP-adjacent files
Two near-identical fixes, one in the Web API error path and one in the public error page.
Magento\Framework\Webapi\ErrorProcessor::_saveFatalErrorReport() before:
$this->directoryWrite->create('report/api'); $reportId = abs((int)(microtime(true) * random_int(100, 1000))); $this->directoryWrite->writeFile('report/api/' . $reportId, $this->serializer->serialize($reportData));
After:
if (is_string($reportData)) { $reportData = str_replace('<?', '< ?', $reportData); } $this->directoryWrite->writeFile( 'report/api/' . $reportId, self::REPORT_EXECUTION_GUARD . PHP_EOL . $this->serializer->serialize($reportData) );
where the guard is the string <?php exit; ?>. pub/errors/processor.php gets the same treatment for the storefront reports under var/report/, with a recursive walk because that report data is an array:
private function sanitizeReportData(array $data): array { array_walk_recursive($data, static function (&$value) { if (is_string($value)) { $value = str_replace('<?', '< ?', $value); } }); return $data; }
and a matching reader that strips the guard back off, so existing report viewing still works:
private function readReportFile(string $reportFile): string { $contents = (string)file_get_contents($reportFile); $guard = self::REPORT_EXECUTION_GUARD; if (strncmp($contents, $guard, strlen($guard)) === 0) { $contents = ltrim(substr($contents, strlen($guard)), "\r\n"); } return $contents; }
The point is not that Magento was executing these files. It was not. The point is that any request that fatals, on /rest/..., on /graphql or on the storefront, gave you a file on disk in a predictable directory whose contents you substantially controlled, including PHP open tags. That is a write primitive parked one misconfiguration away from execution. All it needs is a webserver that serves var/ because someone loosened a location block, an include or file_get_contents anywhere that takes a path fragment, a backup tool that copies var/report somewhere web-visible or a template engine reached through problem one. <?php exit; ?> in front of the payload makes the file inert as a PHP script no matter how it eventually gets included, and neutralising <? to < ? means the body cannot reopen it.
This is the same defence Magento already applies to generated files, and the same reason Laravel writes <?php exit; ?> at the head of its session files. It is cheap and it is unconditional, which is exactly what you want in an error handler that by definition runs when things are already wrong.
The ACL additions, and what they are actually for
Three preview blocks pick up an admin resource constant and a check at the top of _toHtml():
private const ADMIN_RESOURCE = 'Magento_Email::template'; ... protected function _toHtml() { if (!$this->_authorization->isAllowed(self::ADMIN_RESOURCE)) { return ''; }
plus Magento_Newsletter::template on the newsletter template preview and Magento_Newsletter::queue on the queue preview.
It is worth being precise about what this does and does not mean, because it is easy to read a new isAllowed() call as proof that an endpoint was open. It was not. The controllers in front of these blocks were already gated. Magento\Email\Controller\Adminhtml\Email\Template carries public const ADMIN_RESOURCE = 'Magento_Email::template', the newsletter template controller carries Magento_Newsletter::template, and Magento\Newsletter\Controller\Adminhtml\Queue not only declares Magento_Newsletter::queue but overrides _isAllowed(). The admin router requires a session for all of them. Nothing in this patch is a pre-authentication issue, and nobody is hitting these URLs anonymously.
What the block-level check buys you is protection for the case where the block renders somewhere other than behind its own controller. Given problem one, that case is not hypothetical: {{block class="Magento\Email\Block\Adminhtml\Template\Preview"}} in content processed by the CMS filter is the exact scenario, and the block reads type, text and styles off the current request whatever that request happens to be. Moving the authorization check from the route to the block means the check travels with the code that reads the input. That is the right place for it.
What to do
Apply the patch. It is one composer patch per version line, it touches no schema and no DI wiring you own, and there is nothing to configure afterwards. Clear generated code and recompile as usual, because BlockFactory picks up a new constructor argument:
bin/magento setup:upgrade bin/magento setup:di:compile bin/magento cache:flush
The new second constructor argument on BlockFactory is nullable with an object-manager fallback, so a plugin or a preference of your own on that class will keep working. If you have extended BlockFactory or UrlGeneratorFactory and reimplemented createBlock or createUrlGenerator, your override silently keeps the old behaviour. Check for that first, it is the one realistic way to apply this patch and stay vulnerable.
Two things worth doing while you are in there, neither of which is in the patch:
Confirm your webserver cannot serve var/, generated/ or pub/media as anything executable. The nginx sample config gets this right; hand-rolled configs and Apache setups with a loose <Directory> block frequently do not. The report guard makes an inert file, but you would rather not be relying on it.
Grep your own code for the instantiate-then-check pattern, because it is a natural thing to write:
grep -rn --include=*.php -A 4 "objectManager->create(" app/code | grep instanceof
Any hit where the argument came from a request parameter, block data, layout XML from a merchant-editable source or a database column, is the same bug. The fix is the same one line: is_a($className, TheInterface::class, true) before you construct anything.
Why this one is worth reading the diff for
Most Magento security patches are a sanitizer added somewhere obvious. This one is a set of ordering bugs, and ordering bugs are invisible in review because every line involved looks correct. $this->objectManager->create() is correct. instanceof BlockInterface is correct. Putting them in that sequence is the entire vulnerability, and no amount of static analysis was going to flag it.
The same is true of the report writers. Nothing was executing those files, no test would fail, and the code had presumably looked fine to everyone who read it since 2.4.0. It took someone asking a different question: not “is this correct” but “what does this give an attacker that they did not have before”.
Found this useful? Everything here is free and stays that way. If it saved you an afternoon, you can buy me a coffee.