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

permit js files #50

Merged
merged 4 commits into from
Oct 10, 2019
Merged

permit js files #50

merged 4 commits into from
Oct 10, 2019

Conversation

oreoluwa
Copy link
Contributor

@oreoluwa oreoluwa commented Oct 8, 2019

Basically, this seem like a simpler/non-complicated workaround for environment variables haraka/Haraka#2098
As the user may inject the environment variables, manipulate as needed and also be able to load other dynamic configurations within node's runtime.

@oreoluwa oreoluwa changed the title Oreoluwa/permit js files permit js files Oct 8, 2019
@baudehlo
Copy link
Contributor

baudehlo commented Oct 8, 2019

I'm nervous about allowing this, but I'll let others decide on its fate.

@msimerson
Copy link
Member

What am I missing @baudehlo?

For this to be exploited, someone needs to drop a js file onto the filesystem in one of the config directories. If an attacker can do that, haven’t we already lost the security game?

The other way this could be exploited is by embedding a malicious js file in a npm package, but how is this any different/worse than what can be achieved currently with a malicious npm js file?

Maybe I can think of worse things after coffee...

@baudehlo
Copy link
Contributor

baudehlo commented Oct 8, 2019 via email

@oreoluwa
Copy link
Contributor Author

oreoluwa commented Oct 8, 2019

@msimerson those were exactly my thoughts, however, @baudehlo we could also load this within a sandboxed environment that allows access to process.env.HARAKA_*, but i think that'd just be overcomplicating things.
imho, if someone's able to place a malicious js in config, he could also already place it anywhere else and he's probably already able to launch other attacks on the application, through other means

@msimerson
Copy link
Member

As I've let this gestate in my head today, my gut feel has been, golly, it sure was great that Thompson, Richie, and the fellows that invented unix didn't feel obligated to protect us from ourselves. Somehow we've managed to create and administer great and wonderful things without being nannied. Much of the strength and utility comes from being just a fat-finger away from rm -rf / dont/put/a/space/before/a/slash!.

Copy link
Member

@msimerson msimerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@msimerson
Copy link
Member

I'm going to let this sit for a couple more days for feedback, but in the meantime, please also update Changes.md and bump the version in package.json.

@baudehlo
Copy link
Contributor

baudehlo commented Oct 9, 2019 via email

@msimerson msimerson merged commit d9a16c9 into haraka:master Oct 10, 2019
@msimerson
Copy link
Member

@oreoluwa , your change is published on NPM. Thanks for your contribution.

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

Successfully merging this pull request may close these issues.

4 participants