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

Restricting worker env var access should use whitelist, not blacklist #41

Closed
fbbradheintz opened this issue Feb 18, 2020 · 2 comments
Closed
Assignees

Comments

@fbbradheintz
Copy link
Contributor

When restricting access to something with a functionally infinite address space - like the namespace of env vars - blacklisting is a poor practice. Every new env variable that the blacklist doesn't know about becomes a new potential vulnerability. It is impossible for the blacklist to capture all possible invalid inputs that it might want to restrict.

A better practice is to use whitelisting. It is possible to know a priori which env vars the worker needs, and hopefully a runtime failure will alert the user to an attempt to access a non-whitelisted variable.

@shivamshriwas
Copy link
Contributor

Black list env var uses regex for filtering out env variables which are not required or need to be restricted.

These env variables are used to start model worker and changing it to whitelist can easily break functionality as there are too many default env variable as well python virtual env variable which if missed to whitelist will affect funcionality.

Even if still wants to whitelist env var , user can write a regex accordingly.

eg : to blacklist env variables staring with Aws user can use regex ^(aws|AWS)[a-zA-Z0-9_]+$

A user may achive whitelist behaviour by providing a negate of regex

eg: user want to whitelist only env vars starting with aws user can use regex ^(?!aws|AWS)[a-zA-Z0-9_]+$

Considering the above analysis current code for blacklist env vars should not be changed.

@harshbafna
Copy link
Contributor

Closing this due to inactivity and the environment variable restriction can be easily controlled through REGEX. Please reopen if required.

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

4 participants