320739 - plain text email containing link with numerical IP should not be marked as scam

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Mark Jansen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Thunderbird 1.5 RC 1 for Windows, Dutch Build 20051025

Thunderbird thinks too many messages are scams. A plain text message containing an numerical URL is NOT a scam, simply because I can easily SEE this URL is numerical.

The scam feature in itself is great, but it should only apply for HTML messages with hyperlinks to numerical hyperlinks, since in that case people might miss the fact the URL is masked.


Reproducible: Always

Steps to Reproduce:
1. receive plain text mail with numerical hyperlinks
2.
3.

Actual Results:  
Scam every time

Expected Results:  
Not to apply for plain text messages, or at least have the option to do so

Comment 1

[Tal Cohen]

Mark, I tend to disagree; I believe this behavior should stay. Numeric IPs are rarely used in legitimate email; those who do use them are often professionals, whereas the scam protection feature is most useful for newbes, who might not realize the danger of a numeric IP. I suggest closing this bug as "invalid" (or "won't fix").

Comment 2

[Mark Jansen]

(In reply to comment #1)
> Mark, I tend to disagree; I believe this behavior should stay. Numeric IPs are
> rarely used in legitimate email; those who do use them are often professionals,
> whereas the scam protection feature is most useful for newbes, who might not
> realize the danger of a numeric IP. I suggest closing this bug as "invalid" (or
> "won't fix").
> 

Please see http://en.wikipedia.org/wiki/Scam

According to this definition, a scam "is an attempt to intentionally mislead a person or persons". IMHO, you simply can't mislead when begin very open about what you are sending the receipiant(s). In a plain text message, you can simply see the URL the sender is leading you to. 

However, in a HTML message (and only in a HTML message), you could mislead the receipiant by linking to a numerical URL, while the anchored text itself mentions a textual URL.

Comment 3

[Magnus Melin [:mkmelin]]

*** This bug has been marked as a duplicate of 308366 ***

Comment 4

[Mark Jansen]

(In reply to comment #3)
> 
> *** This bug has been marked as a duplicate of 308366 ***
> 

It is not, bug 308366 is only about local IP's and this report is about any IP in plain text messages.

Comment 5

[Magnus Melin [:mkmelin]]

Updating summary, confirming.

Comment 6

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

Of course this patch doesn't affect only plain text messages, but I doubt html messages with IP url as link text are any more likely to be phishing attempts than an other message.

Comment 7

[Scott MacGregor]

Comment on attachment 255117 [details] [diff] [review]
proposed fix

sorry for the delay Magnus, I didn't notice this. Fix looks good.

Comment 8

[Scott MacGregor]

Comment on attachment 255117 [details] [diff] [review]
proposed fix

Magnus, can you make the following change before you check this in:

+       if (aLinkText != aUrl) // visible text is same as URL => not suspicious, even if IP etc.
+       {

** insert the two tests

}

instead of re-assigning failsStaticTests to false.

Comment 9

[Scott MacGregor]

Attachment: slightly different fix

I tweaked Magnus's patch a little bit to eliminate some extra if clauses.

I'm not sure if my :? statement is too hard to read to make it worthwhile.

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 262696 [details] [diff] [review]
slightly different fix

The ending semicolon is missing.

Comment 11

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v2

The ? notation looks good, but the lines sure get hard to read when they are that long. Maybe something like this?

BTW, the method is (partly) oddly off by one space, I'll add a patch to fix that too, if you feel it's worth it.

Comment 12

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v2 (+fix the method alignment)

Aligns the method.

Comment 13

[Scott MacGregor]

Comment on attachment 262742 [details] [diff] [review]
proposed fix, v2 (+fix the method alignment)

cool. Thanks Magnus.

Comment 14

[Nickolay_Ponomarev]

Magnus, I'm a little unclear as to whether you have CVS commit rights or you need someone to check this in for you.

Comment 15

[Magnus Melin [:mkmelin]]

I don't have CVS commit rights yet, the bug is on file though. I'd appreciate if you check it in for me! TIA.

Comment 16

[Nickolay_Ponomarev]

mozilla/mail/base/content/phishingDetector.js  1.28

Comment 17

[Magnus Melin [:mkmelin]]

Comment on attachment 262742 [details] [diff] [review]
proposed fix, v2 (+fix the method alignment)

Asking approval for this, mail only. Removes a whole lot of false positive scam alerts.

Comment 18

[Daniel Veditz [:dveditz]]

This one's for mscott to approve or not.

Comment 19

[Daniel Veditz [:dveditz]]

Comment on attachment 262742 [details] [diff] [review]
proposed fix, v2 (+fix the method alignment)

Past approval point for non-blockers, moving request to 1.8.1.6

Comment 20

[Daniel Veditz [:dveditz]]

mscott: see comment 18