179082 - Forward inline an encrypted message base64-encodes attachments twice

Status: VERIFIED FIXED

(MailNews Core :: Security: S/MIME, defect)

Description

[Kai Engert (:KaiE:)]

Prepare your environemnt for S/Mime encryption.
Compose a new message and choose to encrypt it (but not sign it, for simplicity).
Attach a binary file to the message, like an image.
Send the message to yourself.
Receive the message.
Choose to forward inline.
Turn off encryption and signing.
Send the message to yourself.
Receive the message.

Expected behaviour: The attachments should display correctly.

Actual behaviour: The attachments are broken.


I looked into this a bit more already. What actually happens is that the
incoming message contains the attachments, but they were base64 encoded twice.

If I manually access the message source, and copy an attachment to a file, and
can get to the file by running base64-decode tools twice on it.


The intention of this bug is that we prevent encoding base64 twice when not
necessary.

Comment 1

[Kai Engert (:KaiE:)]

Note that this bug is unrelated to the upcoming fix to bug 161488. The behaviour
is the same, regardless whether you use the fix for bug 161488 or not.

Comment 2

[Jean-Francois Ducarroz]

Does signing the forwarded message is a requirement?

Comment 3

[Kai Engert (:KaiE:)]

It does not matter whether you sign or not, and it does not matter whether you
encrypt or not.

If the initial message was encrypted, had attachments, and you forward inline,
all four combinations
  - not sign, not encrypt
  - sign, not encrypt
  - not sign, encrypt
  - sign, encrypt

produce the incorrect message as described in this bug.

Comment 4

[Jean-Francois Ducarroz]

The image extracted from the original message and saved in the temp directory is
not decoded for some reason! Something wrong in mimedrft.cpp...

Comment 5

[Jean-Francois Ducarroz]

the problem is caused by the line
http://lxr.mozilla.org/seamonkey/source/mailnews/mime/src/mimedrft.cpp#1968

mdd->options->decrypt_p is true therefore we by-pass the decoding! I need to
figure out why we do that test...

Comment 6

[Jean-Francois Ducarroz]

reassign to myself

Comment 7

[Jean-Francois Ducarroz]

Attachment: Proposed fix, v1

This line of code comes from 4.x. Things were little bit different at that
time. I could not find any good reason to haveit anymore in MOzilla! I remove
the test of decrypt_p and everything works well now. I tested a lot of scenario
and cannot find any problem.

Comment 8

[Kai Engert (:KaiE:)]

Comment on attachment 105873 [details] [diff] [review]
Proposed fix, v1

This patch works fine for me.
r=kaie

Comment 9

[(not reading, please use [e-mail] instead)]

Comment on attachment 105873 [details] [diff] [review]
Proposed fix, v1

sr=sspitzer

Comment 10

[Jean-Francois Ducarroz]

Fixed in the trunk

Comment 11

[Charles Rosendahl]

20021203 Trunk Build.

Verified on Linux, Win2k.  Could not verify on OSX as I reopened bug 156958.

Comment 12

[Charles Rosendahl]

Verified on OSX as well.

Verified.

Comment 13

[Kai Engert (:KaiE:)]

*** Bug 190096 has been marked as a duplicate of this bug. ***