Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WDB signatures to allow specific URLs regardless of display text #796

Open
val-ms opened this issue Dec 21, 2022 · 15 comments
Open

WDB signatures to allow specific URLs regardless of display text #796

val-ms opened this issue Dec 21, 2022 · 15 comments

Comments

@val-ms
Copy link
Contributor

val-ms commented Dec 21, 2022

PDB signatures are a watch list to protect certain real-URL domains, by monitoring for suspicious links that say they go to those domains in the display-text, but actually go somewhere else.
WDB signatures are an allow list to prevent phishing heuristics for trusted real-URL + display+URL combinations.

It would be useful to have WDB signatures to allow URLs to click-time protection domains where we are unable to verify what the final/actual domain is that the URL will resolve to.

Our current option is to allow a bunch of TLDs in a regex for the display text, like this:

X:(.+\.)?safelinks.protection.outlook.com:(.+\.)?.*\.(at|be|ca|ch|co\.uk|de|es|fr|ie|in|it|nl|ph|pl|com|com\.(au|cn|hk|my|sg))([/?].*)?

It would be nice to be able to do something like this instead, so we don't have to guess at all of the possible display domain TLD's:

Y:(.+\.)?safelinks.protection.outlook.com

Ref: #771 (comment)

@rzvncj
Copy link
Contributor

rzvncj commented Aug 2, 2023

But how would we decide what constitutes a TLD? Mozilla keeps a list of TLDs. Do we have one in ClamAV?

Otherwise of course guessing at anything other than a one-word suffix (like ".com", or ".net") could get us in trouble, because we can't very well say that safelinks.protection.outlook.com should be allowed to redirect to safelinks.protection.outlook.my-malicious-domain.com.

@val-ms
Copy link
Contributor Author

val-ms commented Aug 2, 2023

The intent is to trust going to the safelinks.protection.outlook.com domain regardless of what the display text says. We would trust safelinks.protection.outlook.com to determine if the site it redirects to is safe.
If we can't figure out where the link will redirect to, we can't override to say "well not not actually safe".

@val-ms
Copy link
Contributor Author

val-ms commented Aug 2, 2023

We wouldn't want to trust safelinks.protection.outlook.com.my-malicious-domain.com, of course. So the whole FQDN would have to be a match.

So perhaps it needs the ^ and $ regex symbols, like:
Y:^(.+\.)?safelinks.protection.outlook.com$

@rzvncj
Copy link
Contributor

rzvncj commented Aug 2, 2023

Ah, so the

X:(.+\.)?safelinks.protection.outlook.com:(.+\.)?.*\.(at|be|ca|ch|co\.uk|de|es|fr|ie|in|it|nl|ph|pl|com|com\.(au|cn|hk|my|sg))([/?].*)?

rule says "if the display text is safelinks.protection.outlook.de and it redirects to safelinks.protection.outlook.com it's fine", not the other way around. Sorry for the misunderstanding - yeah, that's a much easier problem to solve. 😄

@Sanesecurity
Copy link

@val-ms
Copy link
Contributor Author

val-ms commented Aug 2, 2023

Ah, so the

X:(.+\.)?safelinks.protection.outlook.com:(.+\.)?.*\.(at|be|ca|ch|co\.uk|de|es|fr|ie|in|it|nl|ph|pl|com|com\.(au|cn|hk|my|sg))([/?].*)?

rule says "if the display text is safelinks.protection.outlook.de and it redirects to safelinks.protection.outlook.com it's fine", not the other way around. Sorry for the misunderstanding - yeah, that's a much easier problem to solve. 😄

Yup. That's the idea. I'd like an easy way to say that anything that redirects to safelinks.protection.outlook.com is fine.
It should be a fairly easy problem to solve.

@rzvncj
Copy link
Contributor

rzvncj commented Aug 9, 2023

Y appears to be synonymous to X here. Do we now want it to only support the case without : (and break the previous use-case, where it could have been used instead of X)?

@rzvncj
Copy link
Contributor

rzvncj commented Aug 10, 2023

I'm guessing we want to allow both the string1:string2 case, and the string1 case for Y?

@val-ms
Copy link
Contributor Author

val-ms commented Aug 16, 2023

Uh... I had no idea Y already exists. The coincidence that I suggested "Y" in an earlier comment has me chuckling.

It seems like it may do exactly what I was hoping for??? It's not in our documentation and we don't have any examples of it in the daily.wdb.

I found the commit where it was introduced and am just reading what the changes do: 6e3332c

I will will have to do some testing. If it really does what I had been hoping for, we may be able to just update the documentation, add a couple feature tests, and close this ticket.

@rzvncj
Copy link
Contributor

rzvncj commented Aug 16, 2023

Uh... I had no idea Y already exists. The coincidence that I suggested "Y" in an earlier comment has me chuckling.

It seems like it may do exactly what I was hoping for??? It's not in our documentation and we don't have any examples of it in the daily.wdb.

I found the commit where it was introduced and am just reading what the changes do: 6e3332c

I will will have to do some testing. If it really does what I had been hoping for, we may be able to just update the documentation, add a couple feature tests, and close this ticket.

I'd be surprised if it actually works as-is, since this line from the original commit you've linked:

if(( rc = add_pattern(matcher,(const unsigned char*)pattern,flags, buffer[0] == 'Y') ))

no longer exists in the current code. That is, X and Y don't appear to be handled differently.

@rzvncj
Copy link
Contributor

rzvncj commented Aug 16, 2023

grep -IR root_regex_hostonly also returns nothing in the clamav source code directory.

@rzvncj
Copy link
Contributor

rzvncj commented Oct 7, 2023

Hi, any updates on this? Do you want to somehow revert to the old code that used to handle Y, or do we reimplement the functionality?

@rzvncj
Copy link
Contributor

rzvncj commented Feb 12, 2024

Ping? :)

@val-ms
Copy link
Contributor Author

val-ms commented Feb 16, 2024

I'm sorry, I don't have anything else to report. We're down a team member and I haven't been able work on lower priority issues.

@rzvncj
Copy link
Contributor

rzvncj commented Feb 16, 2024

I understand. Sorry to hear about that. Update the issue here if and when you have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants