Approved: only removed from text/plain part

Bug #266220 reported by Jtittsler on 2005-04-12
16
This bug affects 1 person
Affects Status Importance Assigned to Milestone
GNU Mailman
Medium
Mark Sapiro

Bug Description

If someone uses the "Approved: in the first line of the message"
scheme, the line including the list password is only removed from
the first text/plain part. It might be better to iterate over all text/*
parts.

[http://sourceforge.net/tracker/index.php?func=detail&aid=1181161&group_id=103&atid=100103]

Mark Sapiro (msapiro) wrote :

Fixed in CVS for 2.1.7rc1 and 2.2. We still require the
Approve(d): <password> to be the first non-blank line in the
first text/plain part (if it's not an actual header), but
once it's found in the first text/plain part, we remove it
from every text/* part in which we can find it.

Mark Sapiro (msapiro) wrote :

Originator: NO

The fix in 2.1.7 sometimes failed because it didn't decode
quoted-printable and base64 encoded body parts. The fix has now been
enhanced to decode the body part before looking for an Approve(d): line.
This also fixes a related issue of not finding Approve(d): line in the
first text/plain part or getting the password wrong if that part was
encoded.

The fix is in SVN (Mailman/Handlers/Approve.py).

Johnathan Ritzi (jrdioko) wrote :

Bug 871415 was just marked as a duplicate of this. However, I'm running Mailman 2.1.13. Was the fix mentioned by Mark in 2007 not incorporated into the 2.1 branch?

Johnathan Ritzi (jrdioko) wrote :

Sorry, didn't read your comment in the other bug first.

Johnathan Ritzi (jrdioko) wrote :

As requested in bug 871415, the Approved line in the HTML that was not removed was:

<div class=3D"gmail_quote"><div><div><span style=3D"color: rgb(34, 34, 34);=
 font-family: &#39;trebuchet ms&#39;, sans-serif; font-size: 13px; "><span =
class=3D"Apple-style-span" style=3D"color: rgb(0, 0, 0); font-family: arial=
, sans-serif; "><span class=3D"il" style=3D"background-image: initial; back=
ground-attachment: initial; background-origin: initial; background-clip: in=
itial; color: rgb(34, 34, 34); background-color: rgb(255, 255, 255);">Appro=
ved</span><span class=3D"Apple-style-span" style=3D"background-color: rgb(2=
55, 255, 255);">: pass</span><span class=3D"Apple-style-span" style=3D"ba=
ckground-color: rgb(255, 255, 255);">wo</span><=
span class=3D"Apple-style-span" style=3D"background-color: rgb(255, 255, 25=
5);">rd</span></span></span></div>

Pretty horrific HTML by Gmail, I know...

Mark Sapiro (msapiro) wrote :

I'm not going to try to handle that one. The current code looks in the HTML for a match to the pattern constructed by

pattern = name + ':(\xA0|\s|&nbsp;)*' + re.escape(passwd)

which seems to work in most cases. I'm not sure how the above was created, but at a minimum, the text was colored dark grey. If I just create a simple, 'rich formatting' message in gmail, I don't see that. Even with colored text, I see only things like

<span style=3D"color: rgb(51, 51, 51);">Approved: password</span>

I suggest that your poster try creating the post using gmail's "plain text" composition if possible. This (at least in my tests) creates a simple text/plain message.

If the poster feels it is necessary to include whatever fonts, colors, etc. that cause the 'Approved: password' line to be split up into those multiple <span> segments, there's not much I can do.

I could try to detect a case where I can find the Approved: password text if I strip out all the HTML tags, but not if I don't, and refuse to approve the post in that case, but I'm not sure how that should work. Probably, I should reject the post entirely with some reason like "Unable to remove 'Approved: password' text from HTML part". Holding the post doesn't seem right because it could be manually approved which would expose the password.

Johnathan Ritzi (jrdioko) wrote :

I believe all I did in that case was to copy and paste the "Approved" line from a previous email I sent, and it somehow got mutilated with HTML...

I think rejection if you find the password in the stripped-out HTML part is a great idea. The dangerous part of this bug is that a failure in parsing can lead to an admin password being broadcast over email to hundreds of people. It seems like Mailman should either require the header or plain-text email and not even allow HTML emails, or ensure (via some liberal matching) that the password isn't going to get sent out if parsing fails.

Mark Sapiro (msapiro) wrote :

The fix for this bug has been enhanced as discussed in comments #6 and #7. The enhanced fix is committed for release with Mailman 2.1.15.

Changed in mailman:
assignee: nobody → Mark Sapiro (msapiro)
milestone: 2.1-beta → 2.1.15
status: Fix Released → Fix Committed
Mark Sapiro (msapiro) on 2012-06-15
Changed in mailman:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers