1663219 - mimeVerify.onStopRequest attempts to check for a null url, but has already defined it as an empty object

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Mark Banner (:standard8)]

I can only reproduce this with my Conversations add-on in-testing, so I don't think this would be hit normally, however it still looks like a bug in the code:

https://searchfox.org/comm-central/rev/08446144d5725b6e9c2b5f206da75a565910ae55/mail/extensions/openpgp/content/modules/mimeVerify.jsm#423,515

let url = {};
...
if (this.msgUriSpec) {
  url = EnigmailCompat.getUrlFromUriSpec(this.msgUriSpec);
}
...
if (this.uri && url) {

The url check is always going to pass since the empty object is not null.

I spotted this, because when the url hasn't been assigned, the subsequent call to EnigmailURIs.msgIdentificationFromUrl throws.

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1663219_pgp_url_check.patch

Move definition down to where it's first used, and make it null when not set.
We're also already inside if (this.uri)

Comment 2

[Magnus Melin [:mkmelin]]

Attachment: bug1663219_pgp_url_check.patch

Comment 3

[Kai Engert (:KaiE:)]

please uplift correctness fixes to 78 for code consistency

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1a13d435f8ae
fix url check in MimeVerify.onStopRequest. r=PatrickBrunschwig

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9174090 [details] [diff] [review]
bug1663219_pgp_url_check.patch

[Approval Request Comment]
Code correction fix, unclear if it has user impact in practice. Should be safe.

Comment 6

[Rob Lemley [:rjl]]

Comment on attachment 9174090 [details] [diff] [review]
bug1663219_pgp_url_check.patch

[Triage Comment]
Approved for 81.0b3 by wsmwk via Matrix.

Comment 7

[Rob Lemley [:rjl]]

Thunderbird 81.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/8399ffac5f9d

Comment 8

[Wayne Mery (:wsmwk)]

Comment on attachment 9174090 [details] [diff] [review]
bug1663219_pgp_url_check.patch

[Triage Comment]
Approved for esr78.

Comment 9

[Rob Lemley [:rjl]]

Thunderbird 78.2.2:
https://hg.mozilla.org/releases/comm-esr78/rev/3a5eb890b2c1