Cross-compiled nmap 7.70 + NSE crash when running "telnet-brute" #1233

Closed
alexisfacques opened this Issue Jun 6, 2018 · 8 comments

Comments

Projects
None yet
3 participants
@alexisfacques

I'm sorry I couldn't narrow the issue...

Running the telnet-brute script with cross-compiled nmap 7.60 with GCC 6.x for ARM little endian systems worked fine:

$ nmap --version

Nmap version 7.60 ( https://nmap.org )
Platform: arm-buildroot-linux-gnueabi
Compiled with: liblua-5.3.3 openssl-1.0.2o libpcre-8.41 libpcap-1.8.1 nmap-libdnet-1.12 ipv6
Compiled without: libssh2 libz
Available nsock engines: epoll poll select

$ nmap -p 23 --script telnet-brute --script-args userdb=<path/to/users>,passdb=<path/to/pass>,telnet-brute.timeout=8s <target>

Starting Nmap 7.60 ( https://nmap.org ) at <...>
Nmap scan report for <target>
Host is up (0.0011s latency).

PORT   STATE SERVICE
23/tcp open  telnet
| telnet-brute: 
|   Accounts: 
|     admin:admin - Valid credentials
|_  Statistics: Performed 1 guesses in 3 seconds, average tps: 0.3

Running the same script and command, with nmap 7.70 return the following error (with --script-trace output). Related assertion nse_nsock.cc:672 :

nmap -p 23 --script telnet-brute --script-args userdb=<path/to/user>,passdb=<path/to/pass>,telnet-brute.timeout=8s --script-trace <target>
Starting Nmap 7.70 ( https://nmap.org ) at <...>
NSOCK INFO [2.6910s] nsock_trace_handler_callback(): Callback: CONNECT SUCCESS for EID 8 [<target>:23]
NSE: TCP <nmap_host>:52350 > <target>:23 | CONNECT
NSE: TCP <nmap_host>:52350 > <target>:23 | 00000000: 0a                                               

NSOCK INFO [2.6920s] nsock_write(): Write request for 1 bytes to IOD #1 EID 19 [<target>:23]
NSOCK INFO [2.6920s] nsock_trace_handler_callback(): Callback: WRITE SUCCESS for EID 19 [<target>:23]
NSE: TCP <nmap_host>:52350 > <target>:23 | SEND
NSOCK INFO [2.6930s] nsock_read(): Read request from IOD #1 [<target>:23] (timeout: 16000ms) EID 26
NSOCK INFO [2.7040s] nsock_trace_handler_callback(): Callback: READ SUCCESS for EID 26 [<target>:23] (12 bytes): ............
NSE: TCP <nmap_host>:52350 < <target>:23 | 00000000: ff fd 01 ff fd 1f ff fb 01 ff fb 03                         

NSE: TCP <nmap_host>:52350 > <target>:23 | CLOSE
NSOCK INFO [2.7060s] nsock_iod_delete(): nsock_iod_delete (IOD #1)
NSOCK INFO [2.7130s] nsock_iod_new2(): nsock_iod_new (IOD #2)
NSOCK INFO [2.7140s] nsock_connect_tcp(): TCP connection requested to <target>:23 (IOD #2) EID 32
NSOCK INFO [2.7140s] nsock_iod_new2(): nsock_iod_new (IOD #3)
NSOCK INFO [2.7160s] nsock_connect_tcp(): TCP connection requested to <target>:23 (IOD #3) EID 40
NSOCK INFO [2.7160s] nsock_iod_new2(): nsock_iod_new (IOD #4)
NSOCK INFO [2.7180s] nsock_connect_tcp(): TCP connection requested to <target>:23 (IOD #4) EID 48
NSOCK INFO [2.7180s] nsock_iod_new2(): nsock_iod_new (IOD #5)
NSOCK INFO [2.7200s] nsock_connect_tcp(): TCP connection requested to <target>:23 (IOD #5) EID 56
NSOCK INFO [2.7200s] nsock_iod_new2(): nsock_iod_new (IOD #6)
NSOCK INFO [2.7230s] nsock_connect_tcp(): TCP connection requested to <target>:23 (IOD #6) EID 64
NSOCK INFO [2.7240s] nsock_trace_handler_callback(): Callback: CONNECT SUCCESS for EID 32 [<target>:23]
NSE: TCP <nmap_host>:52352 > <target>:23 | CONNECT
NSOCK INFO [2.7240s] nsock_trace_handler_callback(): Callback: CONNECT SUCCESS for EID 40 [<target>:23]
NSE: TCP <nmap_host>:52354 > <target>:23 | CONNECT
NSOCK INFO [2.7240s] nsock_trace_handler_callback(): Callback: CONNECT SUCCESS for EID 48 [<target>:23]
NSE: TCP <nmap_host>:52356 > <target>:23 | CONNECT
NSOCK INFO [2.7240s] nsock_trace_handler_callback(): Callback: CONNECT SUCCESS for EID 56 [<target>:23]
NSE: TCP <nmap_host>:52358 > <target>:23 | CONNECT
NSOCK INFO [2.7730s] nsock_read(): Read request from IOD #6 [<target>:23] (timeout: 8000ms) EID 74
NSOCK INFO [2.7740s] nsock_read(): Read request from IOD #6 [<target>:23] (timeout: 8000ms) EID 82
NSOCK INFO [2.7740s] nsock_read(): Read request from IOD #6 [<target>:23] (timeout: 8000ms) EID 90
NSOCK INFO [2.7740s] nsock_read(): Read request from IOD #6 [<target>:23] (timeout: 8000ms) EID 98
NSOCK INFO [3.7900s] nsock_trace_handler_callback(): Callback: CONNECT SUCCESS for EID 64 [<target>:23]
NSE: TCP <nmap_host>:52360 < <target>:23 | RECEIVE BUF
NSOCK INFO [3.7950s] nsock_trace_handler_callback(): Callback: READ SUCCESS for EID 98 [<target>:23] (12 bytes): ............
NSE: TCP <nmap_host>:52360 < <target>:23 | 00000000: ff fd 01 ff fd 1f ff fb 01 ff fb 03                         

NSOCK INFO [3.7990s] nsock_trace_handler_callback(): Callback: READ SUCCESS for EID 90 [<target>:23] (3 bytes): ...
NSE: TCP <nmap_host>:52360 < <target>:23 | 00000000: 0d 0d 0a                                           

NSOCK INFO [3.8080s] nsock_trace_handler_callback(): Callback: READ SUCCESS for EID 82 [<target>:23] (14 bytes): (none) login: 
NSE: TCP <nmap_host>:52360 < <target>:23 | (none) login: 
nmap: nse_nsock.cc:672: int receive_buf(lua_State*, int, lua_KContext): Assertion `lua_gettop(L) == 7' failed.
Aborted
@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jul 6, 2018

Thanks for the report! I can confirm this is happening, and is not related to cross-compiling or architecture. I've got one fix in for a different assertion,

nmap: nse_nsock.cc:612: void receive_callback(nsock_pool, nsock_event, void*): Assertion `lua_status(L) == LUA_YIELD' failed.

This one was due to either calling close or the script being stopped by some external force while the socket read handler was still waiting. I'm still analyzing why it happened, but the fix is in, which allowed me to get the exact assertion failure you have reported.

Thanks for the report! I can confirm this is happening, and is not related to cross-compiling or architecture. I've got one fix in for a different assertion,

nmap: nse_nsock.cc:612: void receive_callback(nsock_pool, nsock_event, void*): Assertion `lua_status(L) == LUA_YIELD' failed.

This one was due to either calling close or the script being stopped by some external force while the socket read handler was still waiting. I'm still analyzing why it happened, but the fix is in, which allowed me to get the exact assertion failure you have reported.

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jul 7, 2018

Well, this is a tricky one. So a bunch of the NSE socket internals are under the assumption that a socket will be worked on by only a single thread at a time. The socket object has a thread member that holds the Lua state for the running thread. When the callback for an event (connect, read, write, etc.) is called, it uses this member to restore the running thread. However, if two threads (Lua coroutines) share the same socket object, one of them could clobber the running thread. Pinging @batrick for any ideas on how to safely address this.

telnet-brute is triggering the bug because of its "autosize" functions which do fancy things with threads and connections. I think this could be fixed by using the new brute.lua functionality with setReduce instead, but I haven't fully evaluated. Pinging @nnposter or @sergeykhegay to look at this possibility.

We'll get it figured out soon. Very glad you reported it!

Well, this is a tricky one. So a bunch of the NSE socket internals are under the assumption that a socket will be worked on by only a single thread at a time. The socket object has a thread member that holds the Lua state for the running thread. When the callback for an event (connect, read, write, etc.) is called, it uses this member to restore the running thread. However, if two threads (Lua coroutines) share the same socket object, one of them could clobber the running thread. Pinging @batrick for any ideas on how to safely address this.

telnet-brute is triggering the bug because of its "autosize" functions which do fancy things with threads and connections. I think this could be fixed by using the new brute.lua functionality with setReduce instead, but I haven't fully evaluated. Pinging @nnposter or @sergeykhegay to look at this possibility.

We'll get it figured out soon. Very glad you reported it!

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jul 9, 2018

Just adding a few thoughts:

  1. It wouldn't be too difficult to support sharing a socket between threads. The only real problem is when the wrong callback is called based on the event type (read vs connect vs write). But I don't think this is wise because the semantics of multiple read requests would lead to a lot of other subtle bugs.

  2. As far as I can see, the only way to tell that a socket belongs to some other thread as well is to check the existing thread member (set at connect) against the current thread when attempting some other socket operation. I added an assertion for this in 6d72dbb, but it could cause a problem in the case of some legitimate use where mutexes or something are used to prevent concurrent access to the socket between threads.

Just adding a few thoughts:

  1. It wouldn't be too difficult to support sharing a socket between threads. The only real problem is when the wrong callback is called based on the event type (read vs connect vs write). But I don't think this is wise because the semantics of multiple read requests would lead to a lot of other subtle bugs.

  2. As far as I can see, the only way to tell that a socket belongs to some other thread as well is to check the existing thread member (set at connect) against the current thread when attempting some other socket operation. I added an assertion for this in 6d72dbb, but it could cause a problem in the case of some legitimate use where mutexes or something are used to prevent concurrent access to the socket between threads.

@batrick

This comment has been minimized.

Show comment
Hide comment
@batrick

batrick Jul 9, 2018

However, if two threads (Lua coroutines) share the same socket object, one of them could clobber the running thread. Pinging @batrick for any ideas on how to safely address this.

Ya, don't do that! The script/library should be fixed. Library could be modified perhaps to check if the socket currently has a thread operating on it.

batrick commented Jul 9, 2018

However, if two threads (Lua coroutines) share the same socket object, one of them could clobber the running thread. Pinging @batrick for any ideas on how to safely address this.

Ya, don't do that! The script/library should be fixed. Library could be modified perhaps to check if the socket currently has a thread operating on it.

@batrick

This comment has been minimized.

Show comment
Hide comment
@batrick

batrick Jul 9, 2018

It wouldn't be too difficult to support sharing a socket between threads. The only real problem is when the wrong callback is called based on the event type (read vs connect vs write). But I don't think this is wise because the semantics of multiple read requests would lead to a lot of other subtle bugs.

I don't agree. What does it mean to have two threads waiting on a read on a socket? Or, one writing and one reading?

As far as I can see, the only way to tell that a socket belongs to some other thread as well is to check the existing thread member (set at connect) against the current thread when attempting some other socket operation. I added an assertion for this in 6d72dbb, but it could cause a problem in the case of some legitimate use where mutexes or something are used to prevent concurrent access to the socket between threads.

Rather than an assert, I would recommend throwing a Lua error as the script is buggy but it should not cause Nmap to crash. Otherwise, checking if the socket has an operation in-flight is a good idea.

batrick commented Jul 9, 2018

It wouldn't be too difficult to support sharing a socket between threads. The only real problem is when the wrong callback is called based on the event type (read vs connect vs write). But I don't think this is wise because the semantics of multiple read requests would lead to a lot of other subtle bugs.

I don't agree. What does it mean to have two threads waiting on a read on a socket? Or, one writing and one reading?

As far as I can see, the only way to tell that a socket belongs to some other thread as well is to check the existing thread member (set at connect) against the current thread when attempting some other socket operation. I added an assertion for this in 6d72dbb, but it could cause a problem in the case of some legitimate use where mutexes or something are used to prevent concurrent access to the socket between threads.

Rather than an assert, I would recommend throwing a Lua error as the script is buggy but it should not cause Nmap to crash. Otherwise, checking if the socket has an operation in-flight is a good idea.

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jul 9, 2018

Well, it appears things were different than I thought. Sorry, @nnposter, there's nothing wrong with your script. I simplified it to use the brute.lua improvements instead of its own multithreading stuff, and it still triggers the error. Turns out the problem was with the brute.lua improvements, specifically the wrapping of the socket object. Because it was done improperly, any socket calls that were not wrapped explicitly would be called on a reference that just tracked the last socket to be created with brute.new_socket().

A fix is incoming, but @alexisfacques if you want to quickly fix this one script, just change brute.new_socket to nmap.new_socket in telnet-brute.nse. Thanks for the report and your patience!

dmiller-nmap commented Jul 9, 2018

Well, it appears things were different than I thought. Sorry, @nnposter, there's nothing wrong with your script. I simplified it to use the brute.lua improvements instead of its own multithreading stuff, and it still triggers the error. Turns out the problem was with the brute.lua improvements, specifically the wrapping of the socket object. Because it was done improperly, any socket calls that were not wrapped explicitly would be called on a reference that just tracked the last socket to be created with brute.new_socket().

A fix is incoming, but @alexisfacques if you want to quickly fix this one script, just change brute.new_socket to nmap.new_socket in telnet-brute.nse. Thanks for the report and your patience!

@alexisfacques

This comment has been minimized.

Show comment
Hide comment
@alexisfacques

alexisfacques Jul 10, 2018

I'll give that a try, thank you guys for your reactivity!

I'll give that a try, thank you guys for your reactivity!

@nmap-bot nmap-bot closed this in 3c88c17 Jul 10, 2018

nmap-bot pushed a commit that referenced this issue Jul 10, 2018

@dmiller-nmap

This comment has been minimized.

Show comment
Hide comment
@dmiller-nmap

dmiller-nmap Jul 12, 2018

Tracking updates to telnet-brute at #1269

Tracking updates to telnet-brute at #1269

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment