[Bug] RBAC with domain may have potential unxexpected behavior

[Zxilly]

https://github.com/casbin/casbin/blob/d8b8c3d1ad7fc2a9bf03d31955850acb1272f3e1/rbac/default-role-manager/role_manager.go#L489-L495

If we have a domain like name::domain, it may lead to unexpected behavior.

@closetool plz confirm this

[Zxilly]

We can change defaultSeparator to a rarely used unicode code. But change domainAndName to a structure might be a better choice.

[kilosonc]

@Zxilly Yep, we'll discuss about it.

[hsluoyz]

@Zxilly in Casbin, the domain should never contain ::. It's reversed keyword. Like you cannot use for as a variable name in a programming language.

@ErikQQY can you add this naming convention to our docs?

[ErikQQY]

@hsluoyz OK, I am working on it

[Zxilly]

@hsluoyz I believed this is still a potential security problem.
For example, if we have a application use casbin and allow user to define their own domain. It should be user::customdomain in g list.
However, the attacker can create a custom domain admin::anotherdomain, which result to admin::anotherdomain::user in g list. The attacker will get permission from admin::anotherdomain.
We can warn our user to filter :: in domain. Or change domainAndName to a structure can resolve this.
Should we submit this to CVE?

[hsluoyz]

@Zxilly If the user can define domain fields, then isn't he powerful enough to do anything else than just messing with the domain?

[Zxilly]

@hsluoyz like this issue casbin/casbin#493, specially constructed organazation name can lead to permission leakage.

[sagilio]

@hsluoyz I think it is a potential problem too and we can avoid it with some changes.
Casbin.NET version does not have this problem. Because I use Map<domainName, Map<roleName, role>> structure like before and cached all roles and all domains list for pattern feature, it need not domainAndName.

[rrasulzade]

@hsluoyz, here documentation says we can use :: as a separator to distinguish role from user. I'm using the same separator for domains as well. Given the concern above, do I need to update my whole database to replace the separator with a different one? Thanks
https://github.com/casbin/casbin-website/blob/master/docs/RBAC.md#how-to-distinguish-role-from-user

[Zxilly]

@rrasulzade Which language of casbin do you use?

[rrasulzade]

Go

[Zxilly]

@closetool plz notice this