Originally reported to the Drupal security team by
@Berdir →
on 30 March 2022 (#176594), which was subsequently considered a case for security hardening and not a vulnerability.
---
Problem/Motivation
Note: I'm not sure if there is a real issue here, but I thought I'd report it securely so others can double check about this, if ok it can also be fixed in public.
Context: We maintain the site for a {country name} NGO, had a visit from a security scanner or something today. Beside a lot of the usual form select validation errors, I also noticed a lot of these PHP warnings:
User warning: The following theme is missing from the file system: -1" OR 3+619-619-1=0+0+0+1 -- in drupal_get_filename() (line 195 of /app/web/core/includes/bootstrap.inc)
#0 /app/web/core/includes/bootstrap.inc(312): _drupal_error_handler_real(512, 'The following t...', '/app/web/core/i...', 195)
#1 [internal function]: _drupal_error_handler(512, 'The following t...', '/app/web/core/i...', 195, Array)
#2 /app/web/core/includes/bootstrap.inc(195): trigger_error('The following t...', 512)
#3 /app/web/core/includes/bootstrap.inc(214): drupal_get_filename('theme', '-1" OR 3+619-61...')
#4 /app/web/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php(438): drupal_get_path('theme', '-1" OR 3+619-61...')
#5 /app/web/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php(108): Drupal\Core\Asset\LibraryDiscoveryParser->drupalGetPath('theme', '-1" OR 3+619-61...')
#6 /app/web/core/lib/Drupal/Core/Asset/LibraryDiscoveryCollector.php(87): Drupal\Core\Asset\LibraryDiscoveryParser->buildByExtension('-1" OR 3+619-61...')
#7 /app/web/core/lib/Drupal/Core/Asset/LibraryDiscoveryCollector.php(66): Drupal\Core\Asset\LibraryDiscoveryCollector->getLibraryDefinitions('-1" OR 3+619-61...')
#8 /app/web/core/lib/Drupal/Core/Cache/CacheCollector.php(149): Drupal\Core\Asset\LibraryDiscoveryCollector->resolveCacheMiss('-1" OR 3+619-61...')
#9 /app/web/core/lib/Drupal/Core/Asset/LibraryDiscovery.php(44): Drupal\Core\Cache\CacheCollector->get('-1" OR 3+619-61...')
#10 /app/web/core/lib/Drupal/Core/Asset/LibraryDiscovery.php(58): Drupal\Core\Asset\LibraryDiscovery->getLibrariesByExtension('-1" OR 3+619-61...')
#11 /app/web/core/lib/Drupal/Core/Asset/LibraryDependencyResolver.php(68): Drupal\Core\Asset\LibraryDiscovery->getLibraryByName('-1" OR 3+619-61...', NULL)
#12 /app/web/core/lib/Drupal/Core/Asset/LibraryDependencyResolver.php(41): Drupal\Core\Asset\LibraryDependencyResolver->doGetDependencies(Array)
#13 /app/web/core/lib/Drupal/Core/Asset/AssetResolver.php(105): Drupal\Core\Asset\LibraryDependencyResolver->getLibrariesWithDependencies(Array)
#14 /app/web/core/lib/Drupal/Core/Asset/AssetResolver.php(116): Drupal\Core\Asset\AssetResolver->getLibrariesToLoad(Object(Drupal\Core\Asset\AttachedAssets))
#15 /app/web/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php(144): Drupal\Core\Asset\AssetResolver->getCssAssets(Object(Drupal\Core\Asset\AttachedAssets), true)
#16 /app/web/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php(113): Drupal\Core\Ajax\AjaxResponseAttachmentsProcessor->buildAttachmentsCommands(Object(Drupal\Core\Ajax\AjaxResponse), Object(Symfony\Component\HttpFoundation\Request))
#17 /app/web/core/lib/Drupal/Core/EventSubscriber/AjaxResponseSubscriber.php(62): Drupal\Core\Ajax\AjaxResponseAttachmentsProcessor->processAttachments(Object(Drupal\Core\Ajax\AjaxResponse))
#18 [internal function]: Drupal\Core\EventSubscriber\AjaxResponseSubscriber->onResponse(Object(Symfony\Component\HttpKernel\Event\ResponseEvent), 'kernel.response', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#19 /app/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(142): call_user_func(Array, Object(Symfony\Component\HttpKernel\Event\ResponseEvent), 'kernel.response', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#20 /app/vendor/symfony/http-kernel/HttpKernel.php(191): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object(Symfony\Component\HttpKernel\Event\ResponseEvent), 'kernel.response')
#21 /app/vendor/symfony/http-kernel/HttpKernel.php(245): Symfony\Component\HttpKernel\HttpKernel->filterResponse(Object(Drupal\Core\Ajax\AjaxResponse), Object(Symfony\Component\HttpFoundation\Request), 1)
#22 /app/vendor/symfony/http-kernel/HttpKernel.php(91): Symfony\Component\HttpKernel\HttpKernel->handleThrowable(Object(Drupal\Core\Form\FormAjaxException), Object(Symfony\Component\HttpFoundation\Request), 1)
#23 /app/web/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#24 /app/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#25 /app/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#26 /app/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#27 /app/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#28 /app/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#29 /app/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#30 /app/web/core/lib/Drupal/Core/DrupalKernel.php(717): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#31 /app/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#32 {main}
I didn't check in depth, it looks like they are altering the ajax theme state data and somehow we end up using the theme name in there without further validation that this is indeed a valid theme. I would have assumed that we have a hash or something that protects the integrity of the data in there?
---
(Additional comment by @Berdir)
Quick note, noticed a second "creative" attempt:
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[HY000]: General error: 1267 Illegal mix of collations (ascii_general_ci,IMPLICIT) and (utf8mb4_general_ci,COERCIBLE) for operation '=': SELECT "name", "value" FROM "key_value_expire" WHERE "expire" > :now AND "name" IN ( :keys__0 ) AND "collection" = :collection; Array ( [:now] => 1648221064 [:collection] => form [:keys__0] => 1����%2527%2522 ) in Drupal\Core\KeyValueStore\DatabaseStorageExpirable->getMultiple() (line 59 of /app/web/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php).
Looks like non-utf8 characters injected into the form (buld) ID. again, most likely not a real issue, at best some security hardening to check that form/form_build ids are matching an expected format? always makes me slightly uncomfortable if an external party can force database errors/exceptions.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Contributors
- Berdir
- larowlan
- catch
- mcdruid
---
Comment from @catch
I had a quick look at the ::getLibraryByName() issue.
I am pretty sure the problem is that it uses ExtensionResolver::getPath() which uses ExtensionList, which actually discovers extensions in the filesystem on cache misses whether they're installed or not.
Instead, it should use the module or theme list, check both to see if an extension with that name exists, get an extension object, then use Extension::getPathName(). This seems fine for a public issue, but a good bug report.
It would also be possible to add validation that $extension is a valid module or theme pattern in ExtensionPathResolver::getPath() and/or ExtensionList::getPathname() which comes from the already-discovered extension info, seems like that could be a public task/bug too.