sunsetsystems wrote on Wednesday, March 14, 2012:
Probably most developers have used the acl_check() function, defined in library/acl.inc to check if the user has permission to do something. Well it seems to be broken, and has been so for a long time.
This function is supposed to return ‘0’ for no access, ‘1’ for read access, and ‘write’, ‘addonly’ or ‘wsome’ for various levels of write access to whatever the protected item is.
Problem is, the way it’s implemented it always returns a boolean value: true or false. Anything except ‘0’ evaluates to true. This is because it invokes the acl_check() function of the “gacl” class, defined in gacl/gacl.class.php. Therefore any place in OpenEMR that compares the return value to “write” for example will think it matches, because non-empty/non-zero strings evaluate to true in boolean context.
To get the correct return value, acl.inc should invoke acl_return_value() instead.
So, I’ll be committing a fix to do that. It’s possible this may expose some broken code that depends on a boolean result, though I don’t expect much if any of that. Just wanted y’all to be aware of this, and have a chance to comment before I do it.