Page MenuHomePhabricator

debugcommands: create new debugantivirusrunning command
Needs ReviewPublic

Authored by durin42 on Apr 1 2020, 10:46 AM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Summary

This writes the EICAR test file to .hg/cache, in an attempt to trigger
an AV scanner's scanning engine. This should let us (in theory) detect
some cases when a user's slowness is a result of AV scanning.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

durin42 created this revision.Apr 1 2020, 10:46 AM
durin42 updated this revision to Diff 20929.Apr 1 2020, 10:51 AM

I do not understand what the intended side-effect of running this command is supposed to be.

I do not understand what the intended side-effect of running this command is supposed to be.

Well, if an AV engine isn't configured to ignore the user's clone, then this will enrage the AV engine by writing out a "virus".

This started out as a joke, but I kind of came around to the realization that it could be useful. So I'm like....90% sure I think this patch makes sense. :)

Should it delete the file after writing it, or don't bother because the AV may have it locked?

Should it delete the file after writing it, or don't bother because the AV may have it locked?

I figured just leave it since it's tiny, but maybe we should sleep for a while and then remove it?

Should it delete the file after writing it, or don't bother because the AV may have it locked?

I figured just leave it since it's tiny, but maybe we should sleep for a while and then remove it?

That might work. I don't feel too strongly about it, but it seemed weird to leave junk in there and I only realized why as I was writing out the comment. The couple of times I've worked on viruses that evaded detection, file names are what I keyed in on.

Silly question, could make anti virus cough on the mercurial source itself?

mercurial/debugcommands.py
135–138

I think it could be useful to have a clarification of this payload is from an inline comment.

I don't think so, since it's base85-armored and my understanding is that really it's only supposed to trigger in isolation...

marmoute requested changes to this revision.Apr 20 2020, 5:15 AM

The approach seems good, but extra comment and cleanup would be nice.

mercurial/debugcommands.py
139

As @mharbison72 pointed out, it would be nicer to remove the file after it has been scanned.

This revision now requires changes to proceed.Apr 20 2020, 5:15 AM
durin42 updated this revision to Diff 21426.May 18 2020, 1:22 PM
marmoute accepted this revision.Mon, Jun 8, 1:43 PM

Seems overall good. I have some fear over the sleep usage, butit does not seems to depends on test, and I don't know what antivirus looks like.

mercurial/debugcommands.py
143

That seems short ? that might fails on heavy load system. Also, this confused me a bit. How is the antivirus failure supposed to manifest when this code is run? I have no experience with them.

Best guess (given I've never hit an AV problem) is that the AV engine would lose its lunch on the EICAR file and alert the user. I figure if the AV engine isn't picking up on it after 2 seconds then it's probably also not a performance issue for us.

My main question here is : what does "AV engine would lose its lunch" translate in terms of the python runtime ?

Sadly I have no idea on that.

Sadly I have no idea on that.

Do we have a poor windows user that could serve an a guinea pig here ?

pulkit added a subscriber: pulkit.Mon, Jun 29, 4:48 AM

Sadly I have no idea on that.

Do we have a poor windows user that could serve an a guinea pig here ?

Seems like we are not able to find a guinea pig here. How about pushing the patch and finding one in wild? :D

Sadly I have no idea on that.

Do we have a poor windows user that could serve an a guinea pig here ?

Seems like we are not able to find a guinea pig here. How about pushing the patch and finding one in wild? :D

Sounds good to me.