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

Should not use/recommend appending data to EXEs for Windows #843

Closed
AshleyScirra opened this issue Jul 3, 2013 · 11 comments
Closed

Should not use/recommend appending data to EXEs for Windows #843

AshleyScirra opened this issue Jul 3, 2013 · 11 comments
Milestone

Comments

@AshleyScirra
Copy link

Node-webkit's documented way of creating a standalone EXE involves binary-appending the contents of package.nw to the node-webkit executable. This is a bad idea since it makes it likely antivirus software detects false positives. Here's an example of a report of a false positive with bitdefender:

http://www.scirra.com/forum/bitdefender-node-webkit-export-problem-in-r136_topic70230.html

I don't know about other OSs, but having been a Windows dev for a while I do know that binary-appending any data at all to a .exe file is an unusual thing to do. A more normal thing for Windows is to update a binary resource. I'm not surprised extra data at the end of an EXE throws virus reports, since I think it's a rare thing to do, and therefore possibly suspicious. Besides, since there's lots of arbitrary compressed data in the package, that might accidentally match known malicious executable code.

It would be nice if we could just put a file called package.nw next to the EXE and running nw.exe will load that if it sees it. It provides a little extra compression and makes it non-obvious where assets and code are compared to having the full file tree in the same directory as the app. Since it's also a separate plain data file and not part of an executable file, antivirus tools are also probably less likely to have false positives. Finally it doesn't need setting up a shortcut and the EXE can still be run directly, which can help make distribution easier.

Hopefully it's a small change!

@rogerwang
Copy link
Member

We support other ways of redistributing apps: see alternative ways in https://github.com/rogerwang/node-webkit/wiki/How-to-package-and-distribute-your-apps . Is it good for your need?

@AshleyScirra
Copy link
Author

Not really. It's mainly just a convenience thing. We'd prefer not to extract the full file tree, and for convenient export from Construct 2 we don't want to rely on any third party tools. Just having a data file which NW looks for is the best solution for us. I would however consider it a bug that you use or recommend binary appending to EXEs on Windows, though, since you're recommending a method that will make some users see the app as a virus.

@rogerwang
Copy link
Member

We'll support looking for package.nw by default if it's launched with no other argument for app path. And the wiki will be updated to guide users to use options other than binary appending. However, we'll still support the appending in our code.

@AshleyScirra
Copy link
Author

OK, well, that works for us, thanks. Will this make 0.7?

@rogerwang
Copy link
Member

Yeah. It's not a big change.

@Mithgol
Copy link
Contributor

Mithgol commented Jul 3, 2013

We'll support looking for package.nw by default if it's launched with no other argument for app path.

That's a definite improvement, but it could be made even better if node-webkit looked (in its own directory, not to be confused with the current directory unless they happen to be the same) for package.json and then for package.nw even if node-webkit was called with some arguments. And only if both are missing, treat the first argument as the package's pathname.

Otherwise such application (with the default package.nw name) would still be unable to process its arguments directly unless it were appended to the node-webkit's tail (which @AshleyScirra would definitely avoid) or wrapped in a script (such as a .bat file in Windows) that would make its package.nw file (or package.json's folder) the first argument for node-webkit and reorder the rest of the given arguments.

(I have intentionally mentioned package.json here in addition to package.nw because that would fix #826 as well. I even gave package.json some priority over package.nw because that method of distribution would not involve packing and unpacking on each run.)

@miklschmidt
Copy link

+1 to the proposal from @Mithgol.

In most cases it would be much easier, and faster to just ship the code without zipping/appending to .exe. The only reason i've been doing this so far, is to hide the code from the user (feeble attempt at security by obscurity, so much is wrong with that approach), but since i switched to using snapshots to protect the part of my code involving business logic, this is no longer needed. I would love to be able to just throw the code in there as is and distribute it. The extra step to zip up the code to a .nw file might still be needed though, and some people might prefer it just to avoid polluting the directory.

@katanacrimson
Copy link

Another thing to consider - would there be any problems if @trevorlinton's embed:// implementation is also included into node-webkit?

@rogerwang
Copy link
Member

@Mithgol thanks for the proposal. That's better so I'll make the next version as described.

@damianb When @trevorlinton 's patch is merged it'll be taken into account.

@trevorlinton
Copy link

@damianb Unfortunately the embed method wouldn't resolve this issue. @AshleyScirra I downloaded bit defender in a vm and tested this out. It appears its using some windows PE header fields (Exec header) for information it shouldnt. For example it uses the checksum (if set) to validate the package contents. With the checksum not matching the file contents and the Windows PE image size not matching the size of the binary it assumes its malware.

The only way around this would be to add resources in the windows NT/.NET way (such as all html/images/etc) to the executables windows resource files then restamp the image header with a new checksum / image size.

Unfortunately I can't find any cross platform utility to do this (plenty of windows only tools do this though).

Also I should mention non-signed executables get extra scrutiny, if you codesign your application with authenticode, none of this happens.

@trevorlinton
Copy link

This may not be as big of an issue. @rogerwang I can get you a merged/cleaned up patch sometime this week. In addition I can create a nodejs/javascript build tool. This however will require quite a few build time settings specific to Windows (such as entitlement lists, manifest, security requests) in addition would require users to iterate their build number on each build (although this could probably be done automatically for them).

I have some Windows PE Header modifications tools at https://github.com/trevorlinton/node-windowsexe/blob/master/windowspe.js it shouldn't be too difficult to modify (right now it just stamps icons in windows) to stamp the image size/checksum, add manifest/security entitlements in addition to add nw.pak/dlls/resources as official resources into there.

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

No branches or pull requests

6 participants