Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

redis-info.nse breaks when parsing client connections response #2296

Closed
franklymrshankley opened this issue Apr 30, 2021 · 1 comment · Fixed by jjunqueira/nmap#1
Closed

redis-info.nse breaks when parsing client connections response #2296

franklymrshankley opened this issue Apr 30, 2021 · 1 comment · Fixed by jjunqueira/nmap#1
Assignees
Labels

Comments

@franklymrshankley
Copy link

@franklymrshankley franklymrshankley commented Apr 30, 2021

Describe the bug
When running the redis-info.nse script, the script appears to handle empty / null values in the "client connections" responses in a less than graceful manner, and exits with an error and a stack trace. This prevents any other info obtained from the redis server from being displayed.

The following is displayed with -d flag enabled.

NSE: redis-info against 127.0.0.1:6379 threw an error!
/usr/local/bin/../share/nmap/scripts/redis-info.nse:142: table index is nil
stack traceback:
	/usr/local/bin/../share/nmap/scripts/redis-info.nse:142: in local 'process'
	/usr/local/bin/../share/nmap/scripts/redis-info.nse:254: in function </usr/local/bin/../share/nmap/scripts/redis-info.nse:185>
	(...tail calls...)

The problem appears to occur when handling responses to the CLIENT LIST command where the results table contains a blank line, and manifests on line 139 which sets the client IP in the array (client_ips[ip] = true;).

I have debugged and resolved locally by adding the following check just inside the for loop which contains the failing line:

        if not item or 0 == #item then break end

but I accept that there may be more elegant ways to address this.

I have tested this with the redis 2.6 and 6.2.2, and get the same results across both versions.

To Reproduce

$ docker run --rm -ti --name test-redis-info-nse -p 6379:6379 redis
$ nmap -sSV -vv -n -d -p 6379 --script redis-info.nse 127.0.0.1

Expected behavior
Where the redis server allows access, then information should be displayed relating to the server, e.g.

PORT     STATE SERVICE REASON         VERSION
6379/tcp open  redis   syn-ack ttl 64 Redis key-value store 2.6.17 (64 bits)
| redis-info: 
|   Version: 2.6.17
|   Operating System: Linux 3.1.0-28-generic x86_64
|   Architecture: 64 bits
|   Process ID: 1
|   Used CPU (sys): 1.18
|   Used CPU (user): 0.85
|   Connected clients: 28
|   Connected slaves: 0
|   Used memory: 836.97K
|   Role: master
|   Bind addresses: 
|     0.0.0.0
|   Client connections: 
|_    172.17.0.1
Final times for host: srtt: 100 rttvar: 5000  to: 100000

Version info (please complete the following information):

  • OS: linux, various
  • Output of nmap --version:
Nmap version 7.91 ( https://nmap.org )
Platform: x86_64-unknown-linux-gnu
Compiled with: nmap-liblua-5.3.5 openssl-1.1.1f libssh2-1.8.0 libz-1.2.11 libpcre-8.39 libpcap-1.9.1 nmap-libdnet-1.12 ipv6
Compiled without:
Available nsock engines: epoll poll select

Additional context
None.

jjunqueira added a commit to jjunqueira/nmap that referenced this issue Jul 28, 2021
fixes nmap#2296

When the item doesn't exist the script fails and results in "ERROR: Script execution failed (use -d to debug)" as the script output. This patch simply skips over the non-existent item and continues the loop.
jjunqueira added a commit to jjunqueira/nmap that referenced this issue Jul 28, 2021
@nnposter nnposter self-assigned this Apr 25, 2022
@nnposter nnposter added bug NSE script and removed Nmap labels Apr 25, 2022
@nnposter
Copy link

@nnposter nnposter commented Apr 25, 2022

Thank you both for reporting the issue and proposing a fix. A more comprehensive code refactoring was committed as r38378.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@nnposter @franklymrshankley and others