From 92033047a51018285b081cba1d234b704078d9e3 Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Tue, 29 Mar 2022 23:32:33 +0530 Subject: [PATCH 1/4] Python : Add query to detect PAM authorization bypass 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. This PR includes a qhelp describing the issue, a query which detects instances where a call to `pam_acc_mgmt` does not follow a call to `pam_authenticate` and it's corresponding tests. This PR has multiple detections. Some of the public one I can find are : * [CVE-2022-0860](https://nvd.nist.gov/vuln/detail/CVE-2022-0860) found in [cobbler/cobbler](https://www.github.com/cobbler/cobbler) * [fredhutch/motuz](https://www.huntr.dev/bounties/d46f91ca-b8ef-4b67-a79a-2420c4c6d52b/) --- .../Security/CWE-285/PamAuthorization.qhelp | 49 ++++++++++ .../Security/CWE-285/PamAuthorization.ql | 58 +++++++++++ .../Security/CWE-285/PamAuthorizationBad.py | 15 +++ .../Security/CWE-285/PamAuthorizationGood.py | 17 ++++ .../CWE-285/PamAuthorization.expected | 1 + .../Security/CWE-285/PamAuthorization.qlref | 1 + .../query-tests/Security/CWE-285/bad.py | 95 ++++++++++++++++++ .../query-tests/Security/CWE-285/good.py | 97 +++++++++++++++++++ 8 files changed, 333 insertions(+) create mode 100644 python/ql/src/experimental/Security/CWE-285/PamAuthorization.qhelp create mode 100644 python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql create mode 100644 python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py create mode 100644 python/ql/src/experimental/Security/CWE-285/PamAuthorizationGood.py create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.qlref create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-285/bad.py create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-285/good.py 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..e67745cceac1 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql @@ -0,0 +1,58 @@ +/** + * @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 + * @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 + +private class LibPam extends API::Node { + LibPam() { + exists( + API::Node cdll, API::Node find_library, API::Node libpam, API::CallNode cdll_call, + API::CallNode find_lib_call, StrConst str + | + API::moduleImport("ctypes").getMember("CDLL") = cdll and + find_library = API::moduleImport("ctypes.util").getMember("find_library") and + cdll_call = cdll.getACall() and + find_lib_call = find_library.getACall() and + DataFlow::localFlow(DataFlow::exprNode(str), find_lib_call.getArg(0)) and + str.getText() = "pam" and + cdll_call.getArg(0) = find_lib_call and + libpam = cdll_call.getReturn() + | + libpam = this + ) + } + + override string toString() { result = "libpam" } +} + +class PamAuthCall extends API::Node { + PamAuthCall() { exists(LibPam pam | pam.getMember("pam_authenticate") = this) } + + override string toString() { result = "pam_authenticate" } +} + +class PamActMgt extends API::Node { + PamActMgt() { exists(LibPam pam | pam.getMember("pam_acct_mgmt") = this) } + + override string toString() { result = "pam_acct_mgmt" } +} + +from PamAuthCall p, API::CallNode u, Expr handle +where + u = p.getACall() and + handle = u.asExpr().(Call).getArg(0) and + not exists(PamActMgt pam | + DataFlow::localFlow(DataFlow::exprNode(handle), + DataFlow::exprNode(pam.getACall().asExpr().(Call).getArg(0))) + ) +select u, "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..3b06156f5510 --- /dev/null +++ b/python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py @@ -0,0 +1,15 @@ +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) + 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..52c4c8ac6690 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected @@ -0,0 +1 @@ +| bad.py:92:18:92: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/bad.py b/python/ql/test/experimental/query-tests/Security/CWE-285/bad.py new file mode 100644 index 000000000000..84527d6f6fb2 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-285/bad.py @@ -0,0 +1,95 @@ +from ctypes import CDLL, POINTER, Structure, CFUNCTYPE, cast, byref, sizeof +from ctypes import c_void_p, c_size_t, c_char_p, c_char, c_int +from ctypes import memmove +from ctypes.util import find_library + +class PamHandle(Structure): + _fields_ = [ ("handle", c_void_p) ] + + def __init__(self): + Structure.__init__(self) + self.handle = 0 + +class PamMessage(Structure): + """wrapper class for pam_message structure""" + _fields_ = [ ("msg_style", c_int), ("msg", c_char_p) ] + + def __repr__(self): + return "" % (self.msg_style, self.msg) + +class PamResponse(Structure): + """wrapper class for pam_response structure""" + _fields_ = [ ("resp", c_char_p), ("resp_retcode", c_int) ] + + def __repr__(self): + return "" % (self.resp_retcode, self.resp) + +conv_func = CFUNCTYPE(c_int, c_int, POINTER(POINTER(PamMessage)), POINTER(POINTER(PamResponse)), c_void_p) + +class PamConv(Structure): + """wrapper class for pam_conv structure""" + _fields_ = [ ("conv", conv_func), ("appdata_ptr", c_void_p) ] + +# Various constants +PAM_PROMPT_ECHO_OFF = 1 +PAM_PROMPT_ECHO_ON = 2 +PAM_ERROR_MSG = 3 +PAM_TEXT_INFO = 4 +PAM_REINITIALIZE_CRED = 8 + +libc = CDLL(find_library("c")) +libpam = CDLL(find_library("pam")) + +calloc = libc.calloc +calloc.restype = c_void_p +calloc.argtypes = [c_size_t, c_size_t] + +# bug #6 (@NIPE-SYSTEMS), some libpam versions don't include this function +if hasattr(libpam, 'pam_end'): + pam_end = libpam.pam_end + pam_end.restype = c_int + pam_end.argtypes = [PamHandle, c_int] + +pam_start = libpam.pam_start +pam_start.restype = c_int +pam_start.argtypes = [c_char_p, c_char_p, POINTER(PamConv), POINTER(PamHandle)] + +pam_setcred = libpam.pam_setcred +pam_setcred.restype = c_int +pam_setcred.argtypes = [PamHandle, c_int] + +pam_strerror = libpam.pam_strerror +pam_strerror.restype = c_char_p +pam_strerror.argtypes = [PamHandle, c_int] + +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(): + code = 0 + reason = None + + def __init__(self): + pass + + def authenticate(self, username, password, service='login', encoding='utf-8', resetcreds=True): + @conv_func + def my_conv(n_messages, messages, p_response, app_data): + return 0 + + + cpassword = c_char_p(password) + + handle = PamHandle() + conv = PamConv(my_conv, 0) + retval = pam_start(service, username, byref(conv), byref(handle)) + + retval = pam_authenticate(handle, 0) + auth_success = retval == 0 + + return auth_success \ No newline at end of file diff --git a/python/ql/test/experimental/query-tests/Security/CWE-285/good.py b/python/ql/test/experimental/query-tests/Security/CWE-285/good.py new file mode 100644 index 000000000000..e9996c770ed5 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-285/good.py @@ -0,0 +1,97 @@ +from ctypes import CDLL, POINTER, Structure, CFUNCTYPE, cast, byref, sizeof +from ctypes import c_void_p, c_size_t, c_char_p, c_char, c_int +from ctypes import memmove +from ctypes.util import find_library + +class PamHandle(Structure): + _fields_ = [ ("handle", c_void_p) ] + + def __init__(self): + Structure.__init__(self) + self.handle = 0 + +class PamMessage(Structure): + """wrapper class for pam_message structure""" + _fields_ = [ ("msg_style", c_int), ("msg", c_char_p) ] + + def __repr__(self): + return "" % (self.msg_style, self.msg) + +class PamResponse(Structure): + """wrapper class for pam_response structure""" + _fields_ = [ ("resp", c_char_p), ("resp_retcode", c_int) ] + + def __repr__(self): + return "" % (self.resp_retcode, self.resp) + +conv_func = CFUNCTYPE(c_int, c_int, POINTER(POINTER(PamMessage)), POINTER(POINTER(PamResponse)), c_void_p) + +class PamConv(Structure): + """wrapper class for pam_conv structure""" + _fields_ = [ ("conv", conv_func), ("appdata_ptr", c_void_p) ] + +# Various constants +PAM_PROMPT_ECHO_OFF = 1 +PAM_PROMPT_ECHO_ON = 2 +PAM_ERROR_MSG = 3 +PAM_TEXT_INFO = 4 +PAM_REINITIALIZE_CRED = 8 + +libc = CDLL(find_library("c")) +libpam = CDLL(find_library("pam")) + +calloc = libc.calloc +calloc.restype = c_void_p +calloc.argtypes = [c_size_t, c_size_t] + +# bug #6 (@NIPE-SYSTEMS), some libpam versions don't include this function +if hasattr(libpam, 'pam_end'): + pam_end = libpam.pam_end + pam_end.restype = c_int + pam_end.argtypes = [PamHandle, c_int] + +pam_start = libpam.pam_start +pam_start.restype = c_int +pam_start.argtypes = [c_char_p, c_char_p, POINTER(PamConv), POINTER(PamHandle)] + +pam_setcred = libpam.pam_setcred +pam_setcred.restype = c_int +pam_setcred.argtypes = [PamHandle, c_int] + +pam_strerror = libpam.pam_strerror +pam_strerror.restype = c_char_p +pam_strerror.argtypes = [PamHandle, c_int] + +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(): + code = 0 + reason = None + + def __init__(self): + pass + + def authenticate(self, username, password, service='login', encoding='utf-8', resetcreds=True): + @conv_func + def my_conv(n_messages, messages, p_response, app_data): + return 0 + + + cpassword = c_char_p(password) + + 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) + auth_success = retval == 0 + + return auth_success \ No newline at end of file From 785dc1af3c49140a7b4e9ad936ecc80d037b0335 Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Tue, 12 Apr 2022 21:09:05 +0530 Subject: [PATCH 2/4] Include changes from review --- .../Security/CWE-285/PamAuthorization.ql | 56 ++++------- .../Security/CWE-285/PamAuthorizationBad.py | 4 +- .../CWE-285/PamAuthorization.expected | 2 +- .../query-tests/Security/CWE-285/bad.py | 95 ------------------ .../query-tests/Security/CWE-285/good.py | 97 ------------------- .../query-tests/Security/CWE-285/pam_test.py | 59 +++++++++++ 6 files changed, 78 insertions(+), 235 deletions(-) delete mode 100644 python/ql/test/experimental/query-tests/Security/CWE-285/bad.py delete mode 100644 python/ql/test/experimental/query-tests/Security/CWE-285/good.py create mode 100644 python/ql/test/experimental/query-tests/Security/CWE-285/pam_test.py diff --git a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql b/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql index e67745cceac1..3f11728de4e7 100644 --- a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql +++ b/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql @@ -13,46 +13,24 @@ import semmle.python.ApiGraphs import experimental.semmle.python.Concepts import semmle.python.dataflow.new.TaintTracking -private class LibPam extends API::Node { - LibPam() { - exists( - API::Node cdll, API::Node find_library, API::Node libpam, API::CallNode cdll_call, - API::CallNode find_lib_call, StrConst str - | - API::moduleImport("ctypes").getMember("CDLL") = cdll and - find_library = API::moduleImport("ctypes.util").getMember("find_library") and - cdll_call = cdll.getACall() and - find_lib_call = find_library.getACall() and - DataFlow::localFlow(DataFlow::exprNode(str), find_lib_call.getArg(0)) and - str.getText() = "pam" and - cdll_call.getArg(0) = find_lib_call and - libpam = cdll_call.getReturn() - | - libpam = this - ) - } - - override string toString() { result = "libpam" } -} - -class PamAuthCall extends API::Node { - PamAuthCall() { exists(LibPam pam | pam.getMember("pam_authenticate") = this) } - - override string toString() { result = "pam_authenticate" } -} - -class PamActMgt extends API::Node { - PamActMgt() { exists(LibPam pam | pam.getMember("pam_acct_mgmt") = this) } - - override string toString() { result = "pam_acct_mgmt" } +API::Node libPam() { + exists(API::CallNode findLibCall, API::CallNode cdllCall, StrConst str | + findLibCall = API::moduleImport("ctypes.util").getMember("find_library").getACall() and + cdllCall = API::moduleImport("ctypes").getMember("CDLL").getACall() and + DataFlow::localFlow(DataFlow::exprNode(str), findLibCall.getArg(0)) and + str.getText() = "pam" and + cdllCall.getArg(0) = findLibCall + | + result = cdllCall.getReturn() + ) } -from PamAuthCall p, API::CallNode u, Expr handle +from API::CallNode authenticateCall, DataFlow::Node handle where - u = p.getACall() and - handle = u.asExpr().(Call).getArg(0) and - not exists(PamActMgt pam | - DataFlow::localFlow(DataFlow::exprNode(handle), - DataFlow::exprNode(pam.getACall().asExpr().(Call).getArg(0))) + 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 u, "This PAM authentication call may be lead to an authorization bypass." +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 index 3b06156f5510..257f9b997292 100644 --- a/python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py +++ b/python/ql/src/experimental/Security/CWE-285/PamAuthorizationBad.py @@ -1,11 +1,9 @@ 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) 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 index 52c4c8ac6690..cde7271874a0 100644 --- a/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected +++ b/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected @@ -1 +1 @@ -| bad.py:92:18:92:44 | ControlFlowNode for pam_authenticate() | This PAM authentication call may be lead to an authorization bypass. | +| pam_test.py:44:18:44: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/bad.py b/python/ql/test/experimental/query-tests/Security/CWE-285/bad.py deleted file mode 100644 index 84527d6f6fb2..000000000000 --- a/python/ql/test/experimental/query-tests/Security/CWE-285/bad.py +++ /dev/null @@ -1,95 +0,0 @@ -from ctypes import CDLL, POINTER, Structure, CFUNCTYPE, cast, byref, sizeof -from ctypes import c_void_p, c_size_t, c_char_p, c_char, c_int -from ctypes import memmove -from ctypes.util import find_library - -class PamHandle(Structure): - _fields_ = [ ("handle", c_void_p) ] - - def __init__(self): - Structure.__init__(self) - self.handle = 0 - -class PamMessage(Structure): - """wrapper class for pam_message structure""" - _fields_ = [ ("msg_style", c_int), ("msg", c_char_p) ] - - def __repr__(self): - return "" % (self.msg_style, self.msg) - -class PamResponse(Structure): - """wrapper class for pam_response structure""" - _fields_ = [ ("resp", c_char_p), ("resp_retcode", c_int) ] - - def __repr__(self): - return "" % (self.resp_retcode, self.resp) - -conv_func = CFUNCTYPE(c_int, c_int, POINTER(POINTER(PamMessage)), POINTER(POINTER(PamResponse)), c_void_p) - -class PamConv(Structure): - """wrapper class for pam_conv structure""" - _fields_ = [ ("conv", conv_func), ("appdata_ptr", c_void_p) ] - -# Various constants -PAM_PROMPT_ECHO_OFF = 1 -PAM_PROMPT_ECHO_ON = 2 -PAM_ERROR_MSG = 3 -PAM_TEXT_INFO = 4 -PAM_REINITIALIZE_CRED = 8 - -libc = CDLL(find_library("c")) -libpam = CDLL(find_library("pam")) - -calloc = libc.calloc -calloc.restype = c_void_p -calloc.argtypes = [c_size_t, c_size_t] - -# bug #6 (@NIPE-SYSTEMS), some libpam versions don't include this function -if hasattr(libpam, 'pam_end'): - pam_end = libpam.pam_end - pam_end.restype = c_int - pam_end.argtypes = [PamHandle, c_int] - -pam_start = libpam.pam_start -pam_start.restype = c_int -pam_start.argtypes = [c_char_p, c_char_p, POINTER(PamConv), POINTER(PamHandle)] - -pam_setcred = libpam.pam_setcred -pam_setcred.restype = c_int -pam_setcred.argtypes = [PamHandle, c_int] - -pam_strerror = libpam.pam_strerror -pam_strerror.restype = c_char_p -pam_strerror.argtypes = [PamHandle, c_int] - -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(): - code = 0 - reason = None - - def __init__(self): - pass - - def authenticate(self, username, password, service='login', encoding='utf-8', resetcreds=True): - @conv_func - def my_conv(n_messages, messages, p_response, app_data): - return 0 - - - cpassword = c_char_p(password) - - handle = PamHandle() - conv = PamConv(my_conv, 0) - retval = pam_start(service, username, byref(conv), byref(handle)) - - retval = pam_authenticate(handle, 0) - auth_success = retval == 0 - - return auth_success \ No newline at end of file diff --git a/python/ql/test/experimental/query-tests/Security/CWE-285/good.py b/python/ql/test/experimental/query-tests/Security/CWE-285/good.py deleted file mode 100644 index e9996c770ed5..000000000000 --- a/python/ql/test/experimental/query-tests/Security/CWE-285/good.py +++ /dev/null @@ -1,97 +0,0 @@ -from ctypes import CDLL, POINTER, Structure, CFUNCTYPE, cast, byref, sizeof -from ctypes import c_void_p, c_size_t, c_char_p, c_char, c_int -from ctypes import memmove -from ctypes.util import find_library - -class PamHandle(Structure): - _fields_ = [ ("handle", c_void_p) ] - - def __init__(self): - Structure.__init__(self) - self.handle = 0 - -class PamMessage(Structure): - """wrapper class for pam_message structure""" - _fields_ = [ ("msg_style", c_int), ("msg", c_char_p) ] - - def __repr__(self): - return "" % (self.msg_style, self.msg) - -class PamResponse(Structure): - """wrapper class for pam_response structure""" - _fields_ = [ ("resp", c_char_p), ("resp_retcode", c_int) ] - - def __repr__(self): - return "" % (self.resp_retcode, self.resp) - -conv_func = CFUNCTYPE(c_int, c_int, POINTER(POINTER(PamMessage)), POINTER(POINTER(PamResponse)), c_void_p) - -class PamConv(Structure): - """wrapper class for pam_conv structure""" - _fields_ = [ ("conv", conv_func), ("appdata_ptr", c_void_p) ] - -# Various constants -PAM_PROMPT_ECHO_OFF = 1 -PAM_PROMPT_ECHO_ON = 2 -PAM_ERROR_MSG = 3 -PAM_TEXT_INFO = 4 -PAM_REINITIALIZE_CRED = 8 - -libc = CDLL(find_library("c")) -libpam = CDLL(find_library("pam")) - -calloc = libc.calloc -calloc.restype = c_void_p -calloc.argtypes = [c_size_t, c_size_t] - -# bug #6 (@NIPE-SYSTEMS), some libpam versions don't include this function -if hasattr(libpam, 'pam_end'): - pam_end = libpam.pam_end - pam_end.restype = c_int - pam_end.argtypes = [PamHandle, c_int] - -pam_start = libpam.pam_start -pam_start.restype = c_int -pam_start.argtypes = [c_char_p, c_char_p, POINTER(PamConv), POINTER(PamHandle)] - -pam_setcred = libpam.pam_setcred -pam_setcred.restype = c_int -pam_setcred.argtypes = [PamHandle, c_int] - -pam_strerror = libpam.pam_strerror -pam_strerror.restype = c_char_p -pam_strerror.argtypes = [PamHandle, c_int] - -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(): - code = 0 - reason = None - - def __init__(self): - pass - - def authenticate(self, username, password, service='login', encoding='utf-8', resetcreds=True): - @conv_func - def my_conv(n_messages, messages, p_response, app_data): - return 0 - - - cpassword = c_char_p(password) - - 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) - auth_success = retval == 0 - - return auth_success \ No newline at end of file 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..60408ade7225 --- /dev/null +++ b/python/ql/test/experimental/query-tests/Security/CWE-285/pam_test.py @@ -0,0 +1,59 @@ +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 + + +libpam = CDLL(find_library("pam")) + +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 From 6235dc503965deafd3f30ff76e3c7fcd2d5fceb6 Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Wed, 13 Apr 2022 11:44:15 +0200 Subject: [PATCH 3/4] Python: Handle `find_library` assignment to temp variable --- .../src/experimental/Security/CWE-285/PamAuthorization.ql | 7 +++---- .../query-tests/Security/CWE-285/PamAuthorization.expected | 2 +- .../experimental/query-tests/Security/CWE-285/pam_test.py | 6 +++++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql b/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql index 3f11728de4e7..595d1af13a44 100644 --- a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql +++ b/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql @@ -14,12 +14,11 @@ import experimental.semmle.python.Concepts import semmle.python.dataflow.new.TaintTracking API::Node libPam() { - exists(API::CallNode findLibCall, API::CallNode cdllCall, StrConst str | + 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 - DataFlow::localFlow(DataFlow::exprNode(str), findLibCall.getArg(0)) and - str.getText() = "pam" and - cdllCall.getArg(0) = findLibCall + cdllCall.getParameter(0).getAValueReachingRhs() = findLibCall | result = cdllCall.getReturn() ) 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 index cde7271874a0..1b6c23291be9 100644 --- a/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected +++ b/python/ql/test/experimental/query-tests/Security/CWE-285/PamAuthorization.expected @@ -1 +1 @@ -| pam_test.py:44:18:44:44 | ControlFlowNode for pam_authenticate() | This PAM authentication call may be lead to an authorization bypass. | +| 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/pam_test.py b/python/ql/test/experimental/query-tests/Security/CWE-285/pam_test.py index 60408ade7225..966e13cb9911 100644 --- 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 @@ -18,9 +18,13 @@ class PamResponse(Structure): 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)] From ab1252d1967b3a85d0b692241b53990bb12ce55f Mon Sep 17 00:00:00 2001 From: Rasmus Wriedt Larsen Date: Mon, 9 May 2022 14:19:40 +0200 Subject: [PATCH 4/4] Python: Add @precision high for `py/pam-auth-bypass` --- python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql | 1 + 1 file changed, 1 insertion(+) diff --git a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql b/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql index 595d1af13a44..7561dec7f67d 100644 --- a/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql +++ b/python/ql/src/experimental/Security/CWE-285/PamAuthorization.ql @@ -3,6 +3,7 @@ * @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