kylefarris / clamscan

A robust ClamAV virus scanning library supporting scanning files, directories, and streams with local sockets, local/remote TCP, and local clamscan/clamdscan binaries (with failover).
MIT License
236 stars 69 forks source link

Using exit code 1 of exec for positive virus scan #1

Closed dietervds closed 10 years ago

dietervds commented 10 years ago

Hi,

I noticed that when I test the ClamAV exec with a positive file, that we would get an error.

It turns out that when ClamAV detects a virus, it returns an exit code of 1. Exec, by default, detects this as an error, while in this case it isn't. As a result the clamscan library also sees it as an error.
This PR corrects the error handling to interpret err.code===1 as a positive virus match.

If you're wondering, we tested the detection of a virus by using Eicar test file. When you create a file containing the 68 byte string given there, ClamAV (and other scanners) will see it as positive.

Thanks for making this library! It helped me a great deal. Hopefully this PR helps you in return :-)

Cheers,

Dieter

kylefarris commented 10 years ago

Much appreciated Dieter! I'm glad this helped you out. I'll get the updated copy published to NPM as soon as possible.

dietervds commented 10 years ago

My pleasure and thanks! Once it's on npm I'll test it with my own code again.

On Mon, Apr 14, 2014 at 4:45 PM, Kyle Farris notifications@github.comwrote:

Much appreciated Dieter! I'm glad this helped you out. I'll get the updated copy published to NPM as soon as possible.

— Reply to this email directly or view it on GitHubhttps://github.com/kylefarris/clamscan/pull/1#issuecomment-40373614 .

kylefarris commented 10 years ago

Thanks! I've published it to NPM but sometimes it takes a while to refresh on their servers. I should have changed the version from 0.2.0 to 0.2.1 but I accidentally passed 'minor' to the npm version command and it changed the version to 0.3.0. Just a heads up.

dietervds commented 10 years ago

No problem. Don't see a change on npm so far though.
Not sure if it matters, but maybe the version tag in package.json needs to be upped?

kylefarris commented 10 years ago

When you run the npm version patch command, it automatically updates the package.json file. I think the problem was that I needed to upgrade my version of NPM and then re-login. Since I already ran npm version, I went ahead and ran npm publish and seems to have worked. Still don't see the update on NPM itself, though. I'll give it an hour or two and see what happens. I'm still new to all this.

dietervds commented 10 years ago

No worries, I'm very much new to this too :-)

I installed the new version, but it seems we now have another problem I didn't anticipate. The code always ends up in the 'if(err|stderr)' part, because ClamAV spits out a warning about options that are no longer used.

WARNING: Ignoring unsupported option --scan-archive WARNING: Ignoring unsupported option --recursive (-r)

So either we can try and match for 'WARNING' and ignore it, or we can ignore stderr all together. But that last one sounds a bit dangerous :-)

On Tue, Apr 15, 2014 at 4:55 PM, Kyle Farris notifications@github.comwrote:

When you run the npm version patch command, it automatically updates the package.json file. I think the problem was that I needed to upgrade my version of NPM and then re-login. Since I already ran npm version, I went ahead and ran npm publish and seems to have worked. Still don't see the update on NPM itself, though. I'll give it an hour or two and see what happens. I'm still new to all this.

— Reply to this email directly or view it on GitHubhttps://github.com/kylefarris/clamscan/pull/1#issuecomment-40491342 .