Bug 440635 - Scam detector is confused by links created by KMail itself
Summary: Scam detector is confused by links created by KMail itself
Status: CONFIRMED
Alias: None
Product: kmail2
Classification: Applications
Component: message list (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: kdepim bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-08-05 15:36 UTC by Thiago Macieira
Modified: 2021-08-13 05:01 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thiago Macieira 2021-08-05 15:36:05 UTC
SUMMARY
For some emails, the message list shows "This message may be a scam" and shows details saying that it had concluded to be so because the link target was different from the link being displayed. The problem is that this was a plain-text email and the links were generated by KMail. Therefore, the issue is that the display and target are not what's in the plain text source.

For example:

This email contains a link which reads as 'https://codereview.qt-project.org/q/topic:%22api-change-review-6.2%22+(status:open OR status:abandoned' in the text, but actually points to 'https://codereview.qt-project.org/q/topic:'. This is often the case in scam emails to mislead the recipient

STEPS TO REPRODUCE
1. Open the following email body in an KMail

OBSERVED RESULT
Scam warning appears

EXPECTED RESULT
Scam warning should not appear

SOFTWARE/OS VERSIONS
KDE Plasma Version: 5.22.4
KDE Frameworks Version: 5.84.0
Qt Version: 5.15.2 + KDE patches

ADDITIONAL INFORMATION
The email in question was even received in base64 encoding, so it can't have been improperly decoded. Do note it may have been mangled by the mailing list manager, which appended a footer, but other than that the plain text shouldn't have changed.

The offending line is:

Changes: https://codereview.qt-project.org/q/topic:%22api-change-review-6.2%22+(status:open%20OR%20status:abandoned)<https://codereview.qt-project.org/q/topic:"api-change-review-6.2"+(status:open%20OR%20status:abandoned)>

Full email:

MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64

SGkgZXZlcnlvbmUhCgpJdCBzZWVtcyBBUEkgcmV2aWV3IGlzIHN0aWxsIG9uZ29pbmcsIHNlZSBo
dHRwczovL2J1Z3JlcG9ydHMucXQuaW8vYnJvd3NlL1FUQlVHLTk0NDA3CgpQbGVhc2UgY29uY2x1
ZGUgdGhlIHJldmlldyBhc2FwOyB3ZSBzaG91bGQgZ2V0IGFsbCByZXZpZXcgcmVsYXRlZCBjaGFu
Z2VzIGluIGJldGEzIHBhY2thZ2VzCgpiciwKSmFuaQoKX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fXwpGcm9tOiBEZXZlbG9wbWVudCA8ZGV2ZWxvcG1lbnQtYm91bmNlc0Bx
dC1wcm9qZWN0Lm9yZz4gb24gYmVoYWxmIG9mIERhdmlkIFNrb2xhbmQgPGRhdmlkLnNrb2xhbmRA
cXQuaW8+ClNlbnQ6IFR1ZXNkYXksIEp1bmUgMjIsIDIwMjEgMTI6NDMgUE0KVG86IGRldmVsb3Bt
ZW50QHF0LXByb2plY3Qub3JnClN1YmplY3Q6IFtEZXZlbG9wbWVudF0gQVBJIENoYW5nZSBSZXZp
ZXcgZm9yIDYuMgoKSGVsbG8gZXZlcnlvbmUhCgpJbiBhY2NvcmRhbmNlIHRvIFFVSVAtMTAgKGh0
dHA6Ly9xdWlwcy1xdC1pby5oZXJva3VhcHAuY29tL3F1aXAtMDAxMC1BUEktcmV2aWV3Lmh0bWwp
LCBpdCBpcyBhZ2FpbiB0aW1lIGZvciBhIHJldmlldyBvZiB0aGUgY2hhbmdlcyB0byB0aGUgcHVi
bGljIEFQSSBmcm9tIDUuMTUgYW5kIDYuMSB0byA2LjIuIEFzIHlvdSBtYXkga25vdywgYSBmYWly
IG51bWJlciBvZiBtb2R1bGVzIHdlcmUgcmVpbnRyb2R1Y2VkIGluIDYuMiBmcm9tIDUuMTUgYW5k
IHdlcmUgYWRqdXN0ZWQgcXVpdGUgYSBiaXQuIE9uZSBub3RhYmxlIG9taXNzaW9uIGZyb20gdGhl
IHJldmlldyByb3VuZCBpcyBxdG11bHRpbWVkaWEsIHdoaWNoIHdhcyB0aG9yb3VnaGx5IHJld29y
a2VkIGFuZCBoZW5jZSBzdWJqZWN0IHRvIGEgc2VwYXJhdGUgcmV2aWV3IHByb2Nlc3MgKHJlZi4g
TGFycykuCgpSZWxldmFudCBKaXJhOiBodHRwczovL2J1Z3JlcG9ydHMucXQuaW8vYnJvd3NlL1FU
QlVHLTk0NDA3CkNoYW5nZXM6IGh0dHBzOi8vY29kZXJldmlldy5xdC1wcm9qZWN0Lm9yZy9xL3Rv
cGljOiUyMmFwaS1jaGFuZ2UtcmV2aWV3LTYuMiUyMisoc3RhdHVzOm9wZW4lMjBPUiUyMHN0YXR1
czphYmFuZG9uZWQpPGh0dHBzOi8vY29kZXJldmlldy5xdC1wcm9qZWN0Lm9yZy9xL3RvcGljOiJh
cGktY2hhbmdlLXJldmlldy02LjIiKyhzdGF0dXM6b3BlbiUyME9SJTIwc3RhdHVzOmFiYW5kb25l
ZCk+CkFsbCBtb2R1bGVzIGluIDYuMjogaHR0cHM6Ly93aWtpLnF0LmlvL1F0XzYuMi4wX01vZHVs
ZXMKClRoZSBtb3N0IGltcG9ydGFudCBjaGFuZ2VzIGFyZSBhcyB1c3VhbCBxdGJhc2UgYW5kIHF0
ZGVjbGFyYXRpdmUuCgpEbyBhbHNvIG5vdGUgdGhlIDYuMiByZWxlYXNlIHBsYW4sIGFzIHRoaXMg
c2hvdWxkIGJlIGNvbXBsZXRlZCBpbiBhIHRpbWVseSBtYW5uZXI6IGh0dHBzOi8vd2lraS5xdC5p
by9RdF82LjJfUmVsZWFzZQoKQSBmYWlyIG51bWJlciBvZiBtb2R1bGVzIGhhZCBubyBpbnRlcmVz
dGluZyBjaGFuZ2VzIGZyb20gNi4xLCBzbyB0aGV5IGhhdmUgbm8gR2Vycml0IGNoYW5nZSBhdCBh
bGwuIElmIHlvdSBoYXZlIGFueSBxdWVzdGlvbnMgYWJvdXQgYW55IG9mIHRoZSBjaGFuZ2VzLCBs
ZXQgbWUga25vdy4KCkNoZWVycywKCkRhdmlkIFNrb2xhbmQKCl9fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fCkRldmVsb3BtZW50IG1haWxpbmcgbGlzdApEZXZl
bG9wbWVudEBxdC1wcm9qZWN0Lm9yZwpodHRwczovL2xpc3RzLnF0LXByb2plY3Qub3JnL2xpc3Rp
bmZvL2RldmVsb3BtZW50Cg==
Comment 1 Laurent Montel 2021-08-06 05:30:06 UTC
Thanks for bug report.
I will investigate it
Comment 2 Thiago Macieira 2021-08-12 15:09:25 UTC
I can file this as a separate bug report but here's another link. You'll probably get a notification from KMail that the bugzilla email is a scam too.

https://www.google.com/search?q=%5C

The details window will say that link points to 
'https://www.google.com/search?q=/', which is incorrect. It does not. Neither the status bar nor the actual link when opened in the browser suffered the backslash-to-forwardslash transformation. You're incorrectly passing the full, decoded URL through some path clean routine (QDir::cleanPath?). There are at least two mistakes there.
Comment 3 Laurent Montel 2021-08-12 16:09:01 UTC
For the first bug it's not a scam bug it's a problem how we extract url.
I need to fix it.
For second one I have a patch. I need to clean it first.
Comment 4 Laurent Montel 2021-08-12 17:46:39 UTC
Git commit ee84101b36b1ea130c39a5bc9c9b3c471bb4edfb by Laurent Montel.
Committed on 12/08/2021 at 17:45.
Pushed by mlaurent into branch 'release/21.08'.

Fix false positive for url "https://www.google.com/search?q=%5C"

M  +5    -2    messageviewer/src/scamdetection/autotests/scamdetectionwebenginetest.cpp
M  +5    -2    messageviewer/src/scamdetection/scamdetectionwebengine.cpp

https://invent.kde.org/pim/messagelib/commit/ee84101b36b1ea130c39a5bc9c9b3c471bb4edfb
Comment 5 Thiago Macieira 2021-08-12 18:09:37 UTC
Sorry, that can't can't be right. If you have to put the backslashes back, something went wrong before and there may be more.

What were was the value of href and normalizedHref before the toDisplayString call?
Comment 6 Laurent Montel 2021-08-12 19:03:11 UTC
(In reply to Thiago Macieira from comment #5)
> Sorry, that can't can't be right. If you have to put the backslashes back,
> something went wrong before and there may be more.
> 
> What were was the value of href and normalizedHref before the
> toDisplayString call?

"QDEBUG : ScamDetectionWebEngineTest::scamtest(scam5C) 21:01:51.884 scamdetectionwebenginetest(16715) ?[32mMessageViewer::ScamDetectionWebEngine::handleScanPage?[0m text  "https://www.google.com/search?q=%5C"  href  "https://www.google.com/search?q=%5C"  normalizedHref  "https://www.google.com/search?q=%5C""

It was a bug created from a specific url found long time ago:
"<a "
        "href=\"http://g-ecx.images-amazon.com/images/G/01/barcodes/blank003.jpg%5CnUse\">http://g-ecx.images-amazon.com/images/G/01/barcodes/blank003.jpg/"
        "nUse</a>"

=> I fixed it and now all autotest works.
Comment 7 Thiago Macieira 2021-08-12 19:08:17 UTC
> It was a bug created from a specific url found long time ago:
> "<a "
>        
> "href=\"http://g-ecx.images-amazon.com/images/G/01/barcodes/blank003.
> jpg%5CnUse\">http://g-ecx.images-amazon.com/images/G/01/barcodes/blank003.
> jpg/nUse</a>"
> 
> => I fixed it and now all autotest works.

This one should have triggered the warning, because it isn't the same URL. You may want to do the backslash replacement only on the path component instead of the whole URL, if this case is still important.
Comment 8 Laurent Montel 2021-08-13 05:01:23 UTC
(In reply to Thiago Macieira from comment #7)
> > It was a bug created from a specific url found long time ago:
> > "<a "
> >        
> > "href=\"http://g-ecx.images-amazon.com/images/G/01/barcodes/blank003.
> > jpg%5CnUse\">http://g-ecx.images-amazon.com/images/G/01/barcodes/blank003.
> > jpg/nUse</a>"
> > 
> > => I fixed it and now all autotest works.
> 
> This one should have triggered the warning, because it isn't the same URL.
> You may want to do the backslash replacement only on the path component
> instead of the whole URL, if this case is still important.


yep it warns and it's ok in my patch.
Perhaps I need to replace only in path component indeed.