The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.If you change your site name to: " onClick="alert(1); and then click on the logo on the hompage you will get an alert. This issue can be discussed publicly because sitename is something that can only be set by users with the "administer site configuration" which is on the list of permissions that can take over your site http://drupal.org/security-advisory-policy
Here's the line:
$vars['logo_img'] = $vars['logo'] ? '<img src="' . $vars['logo'] . '" alt="' . $vars['site_name'] . '" id="logo" />' : '';
I think switching to theme_image for that should fix the xss and maybe have other benefits: http://api.drupal.org/api/drupal/includes!theme.inc/function/theme_image/7
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | xss_logo_switch_to_theme_image-1540172-2.patch | 923 bytes | IcanDivideBy0 |
| #1 | xss_logo_switch_to_theme_image-1540172-1.patch | 884 bytes | IcanDivideBy0 |
Comments
Comment #1
IcanDivideBy0 commentedHere's the patch
Comment #2
IcanDivideBy0 commentedForgot the id="logo" attribute...
Comment #4
gregglesThe patch doesn't apply for me either, but I'm not sure why.
Visually the change looks right.
Comment #5
IcanDivideBy0 commented#2: xss_logo_switch_to_theme_image-1540172-2.patch queued for re-testing.
Comment #6
gregglesThe xss was fixed with the addition of check_plain in #1395848: Escape site_name in Logo image alt / title text (though the issue doesn't mention the motivation for it). So, anyone who tried to replicate the original issue I posted would be unable to do that on a recent -dev version.
This is no longer about the xss but just about whether or not it's appropriate to use theme_image there. As far as I can tell this patch is a good idea and it keeps out the xss.
I won't RTBC this b/c there may be some reason it doesn't use theme_image but from my perspective it is RTBC.
Comment #7
cellar door commentedHimerus - Good for a 3.2 inclusion?