Closed Bug 320739 Opened 19 years ago Closed 17 years ago

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

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3

People

(Reporter: info, Assigned: mkmelin)

References

Details

(Keywords: fixed1.8.1.8)

Attachments

(1 file, 3 obsolete files)

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
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").
(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.

*** This bug has been marked as a duplicate of 308366 ***
Status: UNCONFIRMED → RESOLVED
Closed: 18 years ago
Resolution: --- → DUPLICATE
(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.
Status: RESOLVED → UNCONFIRMED
Resolution: DUPLICATE → ---
Updating summary, confirming.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Summary: mail scam detection too strict → plain text email containing link with numerical IP should not be marked as scam
Blocks: 370141
No longer blocks: 370141
Blocks: 370141
Assignee: mscott → mkmelin+mozilla
Attached patch proposed fix (obsolete) — Splinter Review
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.
Attachment #255117 - Flags: review?(mscott)
Comment on attachment 255117 [details] [diff] [review]
proposed fix

sorry for the delay Magnus, I didn't notice this. Fix looks good.
Attachment #255117 - Flags: review?(mscott) → review+
Whiteboard: [checkin needed]
Blocks: 378635
QA Contact: front-end
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.
Attached patch slightly different fix (obsolete) — Splinter Review
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.
Attachment #262696 - Flags: superreview?(bienvenu)
Attachment #262696 - Flags: review?(mkmelin+mozilla)
Attachment #262696 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 262696 [details] [diff] [review]
slightly different fix

The ending semicolon is missing.
Attachment #262696 - Flags: review?(mkmelin+mozilla) → review-
Attached patch proposed fix, v2 (obsolete) — Splinter Review
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.
Attachment #255117 - Attachment is obsolete: true
Attachment #262696 - Attachment is obsolete: true
Attachment #262741 - Flags: review?(mscott)
Aligns the method.
Comment on attachment 262742 [details] [diff] [review]
proposed fix, v2 (+fix the method alignment)

cool. Thanks Magnus.
Attachment #262742 - Flags: superreview+
Attachment #262741 - Attachment is obsolete: true
Attachment #262741 - Flags: review?(mscott)
Magnus, I'm a little unclear as to whether you have CVS commit rights or you need someone to check this in for you.
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.
mozilla/mail/base/content/phishingDetector.js  1.28
Status: NEW → RESOLVED
Closed: 18 years ago17 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: --- → Thunderbird 3
Version: unspecified → Trunk
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.
Attachment #262742 - Flags: approval1.8.1.5?
This one's for mscott to approve or not.
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
Attachment #262742 - Flags: approval1.8.1.5? → approval1.8.1.6?
mscott: see comment 18
Attachment #262742 - Flags: approval1.8.1.7? → approval1.8.1.7+
Keywords: fixed1.8.1.7
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: