Pennyw0rth
nxc/protocols/smb/database:447 is_credential_local(self, credential_id) blows up
Describe the bug When executing db.is_credential_local on any id, string or int, local or not, a sqlalchemy stacktrace is produced
To Reproduce Steps to reproduce the behavior i.e.: Add this snippet to nxc/netexec.py line 44 (just after "async def start_run(protocol_obj, args, db, targets):
async def start_run(protocol_obj, args, db, targets):
db.is_credential_local(db.get_credentials()[0][0])
then run anything involving netexec and smb:
poetry run nxc smb showbug --verbose
Resulted in:
Traceback (most recent call last):
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1969, in _exec_single_context
self.dialect.do_execute(
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/engine/default.py", line 922, in do_execute
cursor.execute(statement, parameters)
sqlite3.ProgrammingError: Error binding parameter 1: type 'list' is not supported
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/home/user/gits/NetExec/nxc/netexec.py", line 230, in main
asyncio.run(start_run(protocol_object, args, db, targets))
File "/usr/lib/python3.11/asyncio/runners.py", line 190, in run
return runner.run(main)
^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/asyncio/runners.py", line 118, in run
return self._loop.run_until_complete(task)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/asyncio/base_events.py", line 654, in run_until_complete
return future.result()
^^^^^^^^^^^^^^^
File "/home/user/gits/NetExec/nxc/netexec.py", line 44, in start_run
db.is_credential_local(db.get_credentials()[0][0])
File "/home/user/gits/NetExec/nxc/protocols/smb/database.py", line 453, in is_credential_local
results = self.conn.execute(q).all()
^^^^^^^^^^^^^^^^^^^^
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/orm/session.py", line 2308, in execute
return self._execute_internal(
^^^^^^^^^^^^^^^^^^^^^^^
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/orm/session.py", line 2199, in _execute_internal
result = conn.execute(
^^^^^^^^^^^^^
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1416, in execute
return meth(
^^^^^
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/sql/elements.py", line 517, in _execute_on_connection
return connection._execute_clauseelement(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1639, in _execute_clauseelement
ret = self._execute_context(
^^^^^^^^^^^^^^^^^^^^^^
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1848, in _execute_context
return self._exec_single_context(
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1988, in _exec_single_context
self._handle_dbapi_exception(
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 2344, in _handle_dbapi_exception
raise sqlalchemy_exception.with_traceback(exc_info[2]) from e
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1969, in _exec_single_context
self.dialect.do_execute(
File "/home/user/.cache/pypoetry/virtualenvs/netexec-7SzfTu9v-py3.11/lib/python3.11/site-packages/sqlalchemy/engine/default.py", line 922, in do_execute
cursor.execute(statement, parameters)
sqlalchemy.exc.ProgrammingError: (sqlite3.ProgrammingError) Error binding parameter 1: type 'list' is not supported
[SQL: SELECT hosts.id, hosts.ip, hosts.hostname, hosts.domain, hosts.os, hosts.dc, hosts.smbv1, hosts.signing, hosts.spooler, hosts.zerologon, hosts.petitpotam
FROM hosts
WHERE lower(hosts.id) = lower(?)]
[parameters: ([('sevenkingdoms.local',)],)]
(Background on this error at: https://sqlalche.me/e/20/f405)
Expected behavior No stacktrace, just an error because the target does not resolve:
❯ poetry run nxc smb showbug --verbose
[17:17:58] INFO Error resolving hostname showbug: [Errno -3] Temporary failure in name resolution
NetExec info
- OS: Kali
- Version of nxc: pulled from github, branch main, commit id 669a55f
- Installed from: git
- Running with poetry run nxc from git folder
Additional context I think i have a solution for this bug:
git diff nxc/protocols/smb/database.py
diff --git a/nxc/protocols/smb/database.py b/nxc/protocols/smb/database.py
index f32d7bee..81479322 100755
--- a/nxc/protocols/smb/database.py
+++ b/nxc/protocols/smb/database.py
@@ -446,7 +446,7 @@ class database:
def is_credential_local(self, credential_id):
q = select(self.UsersTable.c.domain).filter(self.UsersTable.c.id == credential_id)
- user_domain = self.conn.execute(q).all()
+ user_domain = self.conn.execute(q).first()[0]
if user_domain:
q = select(self.HostsTable).filter(func.lower(self.HostsTable.c.id) == func.lower(user_domain))
I'll do a PR later. I'm guessing this feature is very seldom used, explaining why no one came across this bug, so there is no rush. I don't think it is a user-facing bug anyway.
@ajanvrin I'm very confused. Why would you add that line in start_run? The stacktrace also tells you what the problem is, a list is being passed in, which isn't expected. This isn't a bug, you're just not using the function correctly.
Hello @Marshall-Hallenbeck,
To clarify, the invalid value "[('sevenkingdoms.local',)]" that is shown in the stacktrace is not the argument that was passed as argument to db.is_credential_local The argument that was passed in this case is just the id of the first credential in the creds db.
There is no point in adding that in "start_run" other than allowing you to reproduce the bug. It's a convenient line for illustration purposes, as the db has already been instantiated, and we can call the function with known-good input.
The line "db.is_credential_local(db.get_credentials()[0][0])" is just a contrived example of calling "db.is_credential_local" with known valid arguments (here i'm passing to it the id of the first credential returned by db.get_credentials, so it is certain that it is a valid input, as it's what the function expects: a credential id).
The bug is not caused by the input to db.is_credential_local, here is where the bug lies, with comments:
# nxc/protocols/smb/database.py line 411
def is_credential_local(self, credential_id):
q = select(self.UsersTable.c.domain).filter(self.UsersTable.c.id == credential_id)
user_domain = self.conn.execute(q).all()
# The previous line executes without error, as the query is correct.
# However, conn.execute(q).all returns a list containing one tuple
# At this point user_domain is a non-empty list of one tuple, which is evaluated as True
if user_domain:
# However the following query does not expect a list,
# it expects a string, this is the source of the bug and the stacktrace
q = select(self.HostsTable).filter(func.lower(self.HostsTable.c.id) == func.lower(user_domain))
results = self.conn.execute(q).all()
return len(results) > 0
Therefore a simple fix consists of changing the following line:
user_domain = self.conn.execute(q).all()
into
user_domain = self.conn.execute(q).first()[0]
first does not return a list of lines but a single table line (a tuple, or rather, in this case a one-uple), and [0] unwraps the one-uple to get the user_domain as a string, which is what I believe was intended.
Again, the reason this bug went unnoticed for so long is that db.is_credential_local is called absolutely nowhere in the entire codebase (/bin/grep -rni is_credential_local yields a single result: where the function is defined)