Not a bug Non-numerical styleId passed to css.php can cause 500 error instead of 404

[Xon]

Affected version

2.1.10 Patch 2

If a non-numerical value is passed via 's' or 'l', then these will clamp to 0. However it is then possible the requested templates may not exist in the master style vs a child style.

$input = $request->filter([
   'css' => 'str',
   's' => 'uint',
   'l' => 'uint',
   'k' => 'str'
]);

For example this request data;

array(4) {
  ["url"] => string(624) "/css.php?css=<redacted>&s=%270%3d1&l=1&d=1597636077&k=1e7730f270c416a5d36b77848c4cb6c22e7c223d"
  ["referrer"] => bool(false)
  ["_GET"] => array(5) {
    ["css"] => string(540) "<redacted>"
    ["s"] => string(4) "'0=1"
    ["l"] => string(1) "1"
    ["d"] => string(10) "1597636077"
    ["k"] => string(40) "1e7730f270c416a5d36b77848c4cb6c22e7c223d"
  }
  ["_POST"] => array(0) {
  }
}

Throws this error;

ErrorException: Template error: Template public:uix_icons.less is unknown
src/XF/Template/Templater.php:618
Generated by: Unknown account

#0 [internal function]: XF\Template\Templater->handleTemplateError()
#1 src/XF/Template/Templater.php(618): trigger_error()
#2 src/XF/Template/Templater.php(580): XF\Template\Templater->getTemplateData()
#3 src/XF/Template/Templater.php(700): XF\Template\Templater->getTemplateMacro()
#4 internal_data/code_cache/templates/l1/s0/public/<redacted>.less.php(8): XF\Template\Templater->callMacro()
#5 src/XF/Template/Templater.php(1315): XF\Template\Templater->{closure}()
#6 src/XF/CssRenderer.php(323): XF\Template\Templater->renderTemplate()
#7 src/XF/CssRenderer.php(250): XF\CssRenderer->renderTemplate()
#8 src/XF/CssRenderer.php(116): XF\CssRenderer->renderTemplates()
#9 src/XF/CssWriter.php(53): XF\CssRenderer->render()
#10 src/addons/SV/RedisCache/XF/CssWriter.php(37): XF\CssWriter->run()
#11 css.php(30): SV\RedisCache\XF\CssWriter->run()
#12 {main}

This is because the include template only exists in the main selectable style, not the master.

 

[Chris D]

This is not something we're able to reproduce so there might be some nuance to this that we're missing.

First it's somewhat confusing to us that the compiled version of the less template is trying to call a macro. I guess this isn't something we explicitly prevent in Less templates but it is unusual:

#4 internal_data/code_cache/templates/l1/s0/public/<redacted>.less.php(8): XF\Template\Templater->callMacro()

Furthermore if the "include template only exists in the main selectable style, not the master" then why is the master style trying to call that template in the first place?

In other words, I'm not actually sure the style ID value makes a difference here. It seems as though there's a fundamental error in that template as it is but I don't think we have a sufficient amount of detail to reproduce this currently.

 

[Xon]

This is a couple of things;

1. A mal-formed request by a bot of some sort, often with what looks to be SQL injection attacks.
2. A 3rd party add-on which has a template modification which injects a template macro which only exists in the site's default style, but not the standard XF master style.

While the template modification is being dumb, it is the malformed request with a broken style id which makes it look like the site generating an 500 error due to an SQL injection attacks instead of returning a sane error code.

 

[Chris D]

I don’t think we’re going to fix that.

The template modification is doing something unexpected and unsupported so the onus would be for the developer who introduced that to fix it.

As such I don’t think we’ll be making any changes here.