Skip to content

Eliminating evals#228

Merged
jstasiak merged 1 commit intonetaddr:masterfrom
KOLANICH-libs:deeval
Apr 15, 2021
Merged

Eliminating evals#228
jstasiak merged 1 commit intonetaddr:masterfrom
KOLANICH-libs:deeval

Conversation

@KOLANICH
Copy link
Contributor

No description provided.

@jstasiak
Copy link
Contributor

Hey, thank you for the contribution. I'm happy to merge this if you modify the commit message to say this is being done to simplify things and because eval() is not actually needed here, not because it's a security risk (because it's not in this case).

@KOLANICH
Copy link
Contributor Author

KOLANICH commented Apr 14, 2021

say this is being done to simplify things and because eval() is not actually needed here, not because it's a security risk (because it's not in this case).

Actually it is, even if it isn't. Because

  1. eval when used where it is really needed is really a security risk;
  2. usage of eval where it is really unneeded is also a security risk, because it disallows to apply a policy "no evals allowed in the code base", which should be understood as "no eval-related security risk is allowed in the code base". And not being able to avoid security risk is a security risk itself.

@jstasiak
Copy link
Contributor

Without going into details I respectfully disagree. I won't merge this change with the current description unless a plausible attack scenario (that's thwarted by this change) is presented.

@KOLANICH
Copy link
Contributor Author

I have fixed the commit describtion.

@jstasiak jstasiak changed the title Eliminating evals, which are security risk Eliminating evals Apr 15, 2021
@jstasiak jstasiak merged commit e84688f into netaddr:master Apr 15, 2021
@jstasiak
Copy link
Contributor

Thank you!

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.

2 participants