New larry skin & literal in Subject header display #3809

Closed
rcubetrac opened this Issue Jun 8, 2012 · 7 comments

Comments

Projects
None yet
1 participant
@rcubetrac

Reported by hh on 8 Jun 2012 03:57 UTC as Trac ticket #1488519

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[    return Q($plugin['output']('valueof']))
!)[($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']($attrib['valueof']]['value'],))) 
!     if ($attrib[== 'subject')
!       return htmlspecialchars($plugin['output']('valueof'])[    else
!       return Q($plugin['output']($attrib['valueof']]['value']);
!)[$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?)

Keywords: larry skin header subject literal
Migrated-From: http://trac.roundcube.net/ticket/1488519

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jun 8, 2012

Comment by @alecpl on 8 Jun 2012 06:50 UTC

What a pretty XSS.

Comment by @alecpl on 8 Jun 2012 06:50 UTC

What a pretty XSS.

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jun 8, 2012

Severity changed by @alecpl on 8 Jun 2012 06:50 UTC

minor => major

Severity changed by @alecpl on 8 Jun 2012 06:50 UTC

minor => major

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jun 8, 2012

Milestone changed by @alecpl on 8 Jun 2012 06:50 UTC

later => 0.8-stable

Milestone changed by @alecpl on 8 Jun 2012 06:50 UTC

later => 0.8-stable

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jun 8, 2012

Comment by @alecpl on 8 Jun 2012 07:24 UTC

Fixed in a7d5e3e. Maybe we should release rc2 now.

Comment by @alecpl on 8 Jun 2012 07:24 UTC

Fixed in a7d5e3e. Maybe we should release rc2 now.

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jun 8, 2012

Status changed by @alecpl on 8 Jun 2012 07:24 UTC

new => closed

Status changed by @alecpl on 8 Jun 2012 07:24 UTC

new => closed

@rcubetrac rcubetrac closed this Jun 8, 2012

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jun 8, 2012

Comment by hh on 8 Jun 2012 07:39 UTC

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 by hh on 8 Jun 2012 07:39 UTC

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...

@rcubetrac

This comment has been minimized.

Show comment
Hide comment
@rcubetrac

rcubetrac Jun 9, 2012

Comment by hh on 9 Jun 2012 02:28 UTC

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

Comment by hh on 9 Jun 2012 02:28 UTC

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

@rcubetrac rcubetrac added this to the 0.8-stable milestone Mar 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment