CRCCheck Feature/Bug - NSIS Forums

dbareis
10th April 2002 11:50 UTC

CRCCheck Feature/Bug
Hi,

I did some checking and found that although a random patch in the installer EXE is detected, adding bytes to the end (changing its file size) isn't.

It makes me worry a bit about what else isn't checked... I had assumed that a CRC check would protect against viruses, I suppose that as well as adding code to the end of the EXE a virus would need to update the EXE header, hopefully this is checked...

Bye
Dennis

Schultz
10th April 2002 13:09 UTC

Well if you look at this page at version nsis17b.exe Justin made this changes to the crc check.
http://www.nullsoft.com/free/nsis/version-history.html

Made CRC not check the first 512 bytes, for better compatibility with virii and with digital signing.

because too many people where complaiing that they NullSoft installers where currupt on there system. not thinking it was a Virus that was causing it etc. and some people i know wouldn't belive that it was a virus on there computer causing it to "currupt the nsis installer" so justin changed what it looked for..

dbareis
13th April 2002 05:47 UTC

Originally posted by Schultz
Well if you look at this page at version nsis17b.exe Justin made this changes to the crc check.
http://www.nullsoft.com/free/nsis/version-history.html

Made CRC not check the first 512 bytes, for better compatibility with virii and with digital signing.

because too many people where complaiing that they NullSoft installers where currupt on there system. not thinking it was a Virus that was causing it etc. and some people i know wouldn't belive that it was a virus on there computer causing it to "currupt the nsis installer" so justin changed what it looked for..

So it sounds like NSIS has NO virus protection and I as a package author can't decide that my installer does. If that is really the case and given that I've proven a virus can add to the end of the file without NSIS complaining and now it doesn't check the EXE header, this is looking very likely then I'm going to have to look for other installer products...

If my product has a virus then I will be blamed, I will not let this happen....

The solution is simple as suggested by me much earlier posts, change the default message to at least suggest the most likely cause is download failure or virus and allow the author to also replace this message.

Bye
Dennis

Repzilon
13th April 2002 18:26 UTC

Problem is identified in source code
From Source\exehead\main.c of version 1.94, at function int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInst,LPSTR lpszCmdParam, int nCmdShow)

#ifdef NSIS_CONFIG_CRC_SUPPORT

       if (no_crc || !(a&FH_FLAGS_CRC))  break; // if first bit is not set, then no crc checking. 


       do_crc++;

        

       left=dbl-4;

        // end crc checking at crc :) this means you can tack shit on the end 

        // and it'll still work.

               

        // this is in case the end part is < 512 bytes. 

       if (l > (DWORD)left) l=(DWORD)left;


>#else//!NSIS_CONFIG_CRC_SUPPORT

        break;

>#endif//!NSIS_CONFIG_CRC_SUPPORT

dbareis
13th April 2002 23:36 UTC

Re: Problem is identified in source code
Quote:

Schultz
14th April 2002 03:14 UTC

Those are defined during compile time because some of the options are done for the exehead. and Justin tries to keep the exehead size as small as possible.. So you can take stuff outta the compile to reduce the exe head even smaller. And makensis i belive just take the already written exehead and just adds the stuff needed onto the end of it. So you can't have stuff definable at runtime since its using an already compiled exehead.

dbareis
16th April 2002 10:57 UTC

Originally posted by Schultz
Those are defined during compile time because some of the options are done for the exehead. and Justin tries to keep the exehead size as small as possible.. So you can take stuff outta the compile to reduce the exe head even smaller. And makensis i belive just take the already written exehead and just adds the stuff needed onto the end of it. So you can't have stuff definable at runtime since its using an already compiled exehead.

Good point.

But while a goal of keeping the header as small as possible is a good one, I think worrying about what is likely to be at most 50-100 bytes is too much (and yes I know they all add up - just like my grocery bill...). Making all NSIS installers open to viruses by default does not seem a good idea to me.

Bye
Dennis

Schultz
16th April 2002 11:15 UTC

Though in my opinion.. As long as you know you are distrubuting an Installer that is virus free on your website or how ever you distribute it.. And the End Users system has the Virus then it is there fault for having the virus.. They should be the ones responsible for having a virus scanner and keeping there virus definitions up to date. But unfortunatly most people that i know of don't do this. Ultimatly you should note that you can only guerentee that the installer is clean if they downloaded it from a "verified source" and since i haven't had a problem with virus's, does other installers such as InstallShield, Ghost(whatever) and all those have a CRC check on the exe file to make sure a virus dosen't alter there's either. (FYI i haven't looked and checked, i am just asking)

Joost Verburg
16th April 2002 19:24 UTC

When you make a freeware application which will be distributed on the internet and will be available on many internet sites, it's good to know that the installer checks for a virus. I don't care when people don't believe that they have a virus, but I don't want that infected versions of the file will be distributed.

But Justin said this about the first 512 bytes:

Not CRCing the first 512 bytes isn't very unsafe. The first 512 bytes are primarily the EXE header and PSP, and if they are corrupted, the installer will likely fail before it gets a check to CRC itself. TRy randomly screwing with the first 512 bytes of a nsis installer and see what happens.

As for compatibility with virii, I think it's best that people's first experiences with NSIS installers are it telling them that they have a virus.

Again, the first 512 bytes, if corrupted, probably aren't going to effect the install process much.

Prove me wrong if you disagree =)

-Justin

So not checking the first 512 bytes is for digital signing or something like that.

justin
17th April 2002 08:56 UTC

Thanks Joost for quoting that old quote.

I'll make an option in 1.97 that lets you do CONFIG_CRC_ANALCHECKING to not allow data to be tacked on the end/not allow the first 512 bytes to change.

It's a matter of preference of how this shit works, so I own't argue the benefits of both.

-Justin

Originally posted by Repzilon
From Source\exehead\main.c of version 1.94, at function int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInst,LPSTR lpszCmdParam, int nCmdShow)

#ifdef NSIS_CONFIG_CRC_SUPPORT

       if (no_crc || !(a&FH_FLAGS_CRC))  break; // if first bit is not set, then no crc checking. 


       do_crc++;

        

       left=dbl-4;

        // end crc checking at crc :) this means you can tack shit on the end 

        // and it'll still work.

               

        // this is in case the end part is < 512 bytes. 

       if (l > (DWORD)left) l=(DWORD)left;


>#else//!NSIS_CONFIG_CRC_SUPPORT

        break;

>#endif//!NSIS_CONFIG_CRC_SUPPORT

Hi,

Thanks for pointing that out.

That code explains the addition of data at end and not the updating of the header but that is probably also relatively easy to find.

However I do enough code hacking (and not enough time to do so), if I ever bother to get the NSIS code I'm likely to want to modify just about everything and I also do not wish to try to keep in sync.

I believe that there are too many "options" like this set during compile time where there would be little if any runtime issue if they were made configurable options. These could have basic doco in a small "advanced" document (little detail as the documentation of all these options is where the main time would be!), but I really don't see why so much configuration is done during the compile.

Bye
Dennis