search

StackStorm / st2-auth-ldap

LDAP backend for user authentication in StackStorm. Previously LDAP backend for EWC.
https://docs.stackstorm.com/authentication.html
Apache License 2.0
5 stars 12 forks source link

Fix group dn check security issue #29

Closed Kami closed 7 years ago

Kami commented 7 years ago

This pull request cherry-picks fix for the group membership check related security issue - #28.

This way we can merge it into master and v2.2 branch separately and include it in v2.2.1 release.

Kami commented 7 years ago

Noticed another thing while looking at the code - we don't escape user provided input (username) which could potentially cause issues.

Unlike SQL and other injections, the way the code and second filter query is structured, I don't think it can actually be abused in an un-safe manner, but the be on the safe-side, we should still escape the user provided values used in the filter queries (notably, username since password is not directly used in a query).

I believe simple ldap.filter.escape_filter_chars and ldap.filter.filter_format call should do it.

Will look into it more and make a change next week.

Kami commented 7 years ago

I pushed changes which fix potential filter injection and privilege escalation issue - f82097a43021d752e50821b780968a84c10513b9, 4fcf4c51021ccdd8c66644a5d59562ee1e07e61e, 7b53bc334fc8bd4893130c61d38cda807992cc5f.

The way the code is currently structured exploiting it wouldn't be totally trivial (e.g. if you just pass in * for username because we have a check in code and if bail out if query returns multiple results), but there are probably ways around that with more complex queries (filters).