search

haraka / haraka-config

Haraka config file loader and parser
https://www.npmjs.com/package/haraka-config
MIT License
10 stars 13 forks source link

permit js files #50

Closed oreoluwa closed 5 years ago

oreoluwa commented 5 years ago

Basically, this seem like a simpler/non-complicated workaround for environment variables https://github.com/haraka/Haraka/issues/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.

baudehlo commented 5 years ago

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

msimerson commented 5 years ago

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 commented 5 years ago

Those, plus basically the reason I sandboxed plugins (a decision we continue to debate the usefulness of) was to prevent people from shooting themselves in the foot.

Like I said, I don't really feel the need to veto it though. If the consensus is that it's useful and not problematic, then put it in.

On Tue, Oct 8, 2019 at 11:56 AM Matt Simerson notifications@github.com wrote:

What am I missing @baudehlo https://github.com/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...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/haraka/haraka-config/pull/50?email_source=notifications&email_token=AAFBWY7BAHSXKB4WY3CGIXLQNSUQ3A5CNFSM4I6PRYZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAUVVLA#issuecomment-539581100, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFBWY5SQQNZW6K47EFODPDQNSUQ3ANCNFSM4I6PRYZQ .

oreoluwa commented 5 years ago

@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 commented 5 years ago

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!.

msimerson commented 5 years ago

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 commented 5 years ago

Yeah let's not let the horrible Node sandboxing infect any other parts of the Haraka codebase. One day I'd like to remove it from plugins too.

On Wed, Oct 9, 2019 at 1:08 AM Matt Simerson notifications@github.com wrote:

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.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/haraka/haraka-config/pull/50?email_source=notifications&email_token=AAFBWY2BFU6CXRWJJXH55Q3QNVRMRA5CNFSM4I6PRYZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAWTDLI#issuecomment-539832749, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFBWY6ZHDU47J7ODJVCN4DQNVRMRANCNFSM4I6PRYZQ .

msimerson commented 5 years ago

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