Old 10th April 2002, 11:50   #1
dbareis
Member
 
Join Date: Apr 2001
Posts: 77
Unhappy 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
dbareis is offline   Reply With Quote
Old 10th April 2002, 13:09   #2
Schultz
Member
 
Join Date: Oct 2001
Posts: 71
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..
Schultz is offline   Reply With Quote
Old 13th April 2002, 05:47   #3
dbareis
Member
 
Join Date: Apr 2001
Posts: 77
Thumbs down

Quote:
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
dbareis is offline   Reply With Quote
Old 13th April 2002, 18:26   #4
Repzilon
Member
 
Join Date: Apr 2001
Posts: 51
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)

PHP Code:
#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 (> (DWORD)leftl=(DWORD)left;

#else//!NSIS_CONFIG_CRC_SUPPORT
        
break;
#endif//!NSIS_CONFIG_CRC_SUPPORT 
Repzilon is offline   Reply With Quote
Old 13th April 2002, 23:36   #5
dbareis
Member
 
Join Date: Apr 2001
Posts: 77
Re: Problem is identified in source code

Quote:
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)

PHP Code:
#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 (> (DWORD)leftl=(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
dbareis is offline   Reply With Quote
Old 14th April 2002, 03:14   #6
Schultz
Member
 
Join Date: Oct 2001
Posts: 71
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.
Schultz is offline   Reply With Quote
Old 16th April 2002, 10:57   #7
dbareis
Member
 
Join Date: Apr 2001
Posts: 77
Quote:
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
dbareis is offline   Reply With Quote
Old 16th April 2002, 11:15   #8
Schultz
Member
 
Join Date: Oct 2001
Posts: 71
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)
Schultz is offline   Reply With Quote
Old 16th April 2002, 19:24   #9
Joost Verburg
NSIS MUI Dev
 
Join Date: Nov 2001
Posts: 3,717
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:
Quote:
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.
Joost Verburg is offline   Reply With Quote
Old 17th April 2002, 08:56   #10
justin
Moderator Alumni
 
Join Date: Apr 2000
Location: USA
Posts: 315
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
justin is offline   Reply With Quote
Reply
Go Back   Winamp & Shoutcast Forums > Developer Center > NSIS Discussion

Thread Tools Search this Thread
Search this Thread:

Advanced Search
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump