diff --git a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.qhelp b/python/ql/src/experimental/Security/CWE-285/PamAuthorization.qhelp new file mode 100644 index 000000000000..8e0f829f33ee --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-285/PamAuthorization.qhelp @@ -0,0 +1,49 @@ + + + +

+ Using only a call to + pam_authenticate + to check the validity of a login can lead to authorization bypass vulnerabilities. +

+

+ A + pam_authenticate + only verifies the credentials of a user. It does not check if a user has an appropriate authorization to actually login. This means a user with a expired login or a password can still access the system. +

+ +
+ + +

+ A call to + pam_authenticate + should be followed by a call to + pam_acct_mgmt + to check if a user is allowed to login. +

+
+ + +

+ In the following example, the code only checks the credentials of a user. Hence, in this case, a user expired with expired creds can still login. This can be verified by creating a new user account, expiring it with + chage -E0 `username` + and then trying to log in. +

+ + +

+ This can be avoided by calling + pam_acct_mgmt + call to verify access as has been done in the snippet shown below. +

+ +
+ + +
  • + Man-Page: + pam_acct_mgmt +
  • +
    +
    \ No newline at end of file diff --git a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql b/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql new file mode 100644 index 000000000000..7561dec7f67d --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql @@ -0,0 +1,36 @@ +/** + * @name Authorization bypass due to incorrect usage of PAM + * @description Using only the `pam_authenticate` call to check the validity of a login can lead to a authorization bypass. + * @kind problem + * @problem.severity warning + * @precision high + * @id py/pam-auth-bypass + * @tags security + * external/cwe/cwe-285 + */ + +import python +import semmle.python.ApiGraphs +import experimental.semmle.python.Concepts +import semmle.python.dataflow.new.TaintTracking + +API::Node libPam() { + exists(API::CallNode findLibCall, API::CallNode cdllCall | + findLibCall = API::moduleImport("ctypes.util").getMember("find_library").getACall() and + findLibCall.getParameter(0).getAValueReachingRhs().asExpr().(StrConst).getText() = "pam" and + cdllCall = API::moduleImport("ctypes").getMember("CDLL").getACall() and + cdllCall.getParameter(0).getAValueReachingRhs() = findLibCall + | + result = cdllCall.getReturn() + ) +} + +from API::CallNode authenticateCall, DataFlow::Node handle +where + authenticateCall = libPam().getMember("pam_authenticate").getACall() and + handle = authenticateCall.getArg(0) and + not exists(API::CallNode acctMgmtCall | + acctMgmtCall = libPam().getMember("pam_acct_mgmt").getACall() and + DataFlow::localFlow(handle, acctMgmtCall.getArg(0)) + ) +select authenticateCall, "This PAM authentication call may be lead to an authorization bypass." diff --git a/python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py b/python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py new file mode 100644 index 000000000000..257f9b997292 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py @@ -0,0 +1,13 @@ +def authenticate(self, username, password, service='login', encoding='utf-8', resetcreds=True): + libpam = CDLL(find_library("pam")) + pam_authenticate = libpam.pam_authenticate + pam_authenticate.restype = c_int + pam_authenticate.argtypes = [PamHandle, c_int] + + + handle = PamHandle() + conv = PamConv(my_conv, 0) + retval = pam_start(service, username, byref(conv), byref(handle)) + + retval = pam_authenticate(handle, 0) + return retval == 0 \ No newline at end of file diff --git a/python/ql/src/experimental/Security/CWE-285/PamAuthorizationGood.py b/python/ql/src/experimental/Security/CWE-285/PamAuthorizationGood.py new file mode 100644 index 000000000000..0f047c6ac659 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-285/PamAuthorizationGood.py @@ -0,0 +1,17 @@ +def authenticate(self, username, password, service='login', encoding='utf-8', resetcreds=True): + libpam = CDLL(find_library("pam")) + pam_authenticate = libpam.pam_authenticate + pam_acct_mgmt = libpam.pam_acct_mgmt + pam_authenticate.restype = c_int + pam_authenticate.argtypes = [PamHandle, c_int] + pam_acct_mgmt.restype = c_int + pam_acct_mgmt.argtypes = [PamHandle, c_int] + + handle = PamHandle() + conv = PamConv(my_conv, 0) + retval = pam_start(service, username, byref(conv), byref(handle)) + + retval = pam_authenticate(handle, 0) + if retval == 0: + retval = pam_acct_mgmt(handle, 0) + return retval == 0 \ No newline at end of file diff --git a/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected b/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected new file mode 100644 index 000000000000..1b6c23291be9 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected @@ -0,0 +1 @@ +| pam_test.py:48:18:48:44 | ControlFlowNode for pam_authenticate() | This PAM authentication call may be lead to an authorization bypass. | diff --git a/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.qlref b/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.qlref new file mode 100644 index 000000000000..38fac298b1e3 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-285/PamAuthorization.ql diff --git a/python/ql/test/experimental/query-tests/Security/CWE-285/pam_test.py b/python/ql/test/experimental/query-tests/Security/CWE-285/pam_test.py new file mode 100644 index 000000000000..966e13cb9911 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-285/pam_test.py @@ -0,0 +1,63 @@ +from ctypes import CDLL, POINTER, Structure, byref +from ctypes import c_char_p, c_int +from ctypes.util import find_library + + +class PamHandle(Structure): + pass + + +class PamMessage(Structure): + pass + + +class PamResponse(Structure): + pass + + +class PamConv(Structure): + pass + +# this is normal way to do things +libpam = CDLL(find_library("pam")) + +# but we also handle assignment to temp variable +temp = find_library("pam") +libpam = CDLL(temp) + +pam_start = libpam.pam_start +pam_start.restype = c_int +pam_start.argtypes = [c_char_p, c_char_p, POINTER(PamConv), POINTER(PamHandle)] + +pam_authenticate = libpam.pam_authenticate +pam_authenticate.restype = c_int +pam_authenticate.argtypes = [PamHandle, c_int] + +pam_acct_mgmt = libpam.pam_acct_mgmt +pam_acct_mgmt.restype = c_int +pam_acct_mgmt.argtypes = [PamHandle, c_int] + + +class pam(): + + def authenticate_bad(self, username, service='login'): + handle = PamHandle() + conv = PamConv(None, 0) + retval = pam_start(service, username, byref(conv), byref(handle)) + + retval = pam_authenticate(handle, 0) + auth_success = retval == 0 + + return auth_success + + def authenticate_good(self, username, service='login'): + handle = PamHandle() + conv = PamConv(None, 0) + retval = pam_start(service, username, byref(conv), byref(handle)) + + retval = pam_authenticate(handle, 0) + if retval == 0: + retval = pam_acct_mgmt(handle, 0) + auth_success = retval == 0 + + return auth_success