Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#1488519 closed Bugs (fixed)

New larry skin & literal in Subject header display

Reported by: hh Owned by:
Priority: 1 - Highest Milestone: 0.8-stable
Component: Security Version: 0.8-rc
Severity: major Keywords: larry skin header subject literal
Cc: thomas@…

Description

One of the good things about the larry skin from 0.8-rc is that
it has the following in the message.html template to make
the Subject line stand out:

<h2 class="subject"><roundcube:object name="messageHeaders" valueOf="subject" /></h2>
<roundcube:object name="messageHeaders" class="headers-table" addicon="/images/addcontact.png" exclude="subject" />

Because of this, however, HTML special characters appearing in the Subject
line remain as they are - i.e., no conversion to HTML entities.

TO REPRODUCE

Try creating email with a Subject line like:

Subject: &copy; <html> &amp; "quoted"

and view it with larry.

Just experimenting:

*** func.inc.orig Fri May 18 16:06:54 2012
--- func.inc  Fri Jun  8 03:18:05 2012
***************
*** 947,953 ****
  
    // single header value is requested
    if (!empty($attrib['valueof']))
!     return Q($plugin['output'][$attrib['valueof']]['value'], ($hkey == 'subject' ? 'strict' : 'show'));
  
    // compose html table
    $table = new html_table(array('cols' => 2));
--- 947,956 ----
  
    // single header value is requested
    if (!empty($attrib['valueof'])) 
!     if ($attrib['valueof'] == 'subject')
!       return htmlspecialchars($plugin['output'][$attrib['valueof']]['value']);
!     else
!       return Q($plugin['output'][$attrib['valueof']]['value'], 'show');
  
    // compose html table
    $table = new html_table(array('cols' => 2));

Notes:

  • Q() with strict doesn't seem to work with entities like &amp;.
  • Tested with PHP 5.3.10 with reasonable defaults, along with DOM/XML API 20031129, libxml 2.6.17, and libxslt 1.1.2.

(My question is, which file should be modified to get the Subject line right - skins/larry/templates/message.html -or- program/steps/mail/func.inc -or- somewhere else?)

Change History (4)

comment:1 Changed 2 years ago by alec

  • Component changed from User Interface to Security issue
  • Milestone changed from later to 0.8-stable
  • Priority changed from 7 to 1 - Highest
  • Severity changed from minor to major

What a pretty XSS.

comment:2 follow-up: Changed 2 years ago by alec

  • Cc thomas@… added
  • Resolution set to fixed
  • Status changed from new to closed

Fixed in a7d5e3e8. Maybe we should release rc2 now.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 2 years ago by hh

Just noticed that the same thing happens in the message list pane with the default skin as well, so I guess the issue has been around for some time...

comment:4 in reply to: ↑ 3 Changed 2 years ago by hh

Replying to hh:

Just noticed that the same thing happens in the message list pane with the default skin as well, so I guess the issue has been around for some time...

I apologize. I stand corrected. Please just disregard my previous comment. That hasn't been an XSS issue. I was confused. I have created a separate feature request somewhat related to the Subject line issue: Ticket #1488523

Note: See TracTickets for help on using tickets.