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

LGTM.com issues reported against Nmap #1834

Closed
guyharris opened this issue Nov 19, 2019 · 4 comments
Closed

LGTM.com issues reported against Nmap #1834

guyharris opened this issue Nov 19, 2019 · 4 comments

Comments

@guyharris
Copy link

Originally mentioned in passing in #1827, LGTM.com has scanned Nmap and found some issues.

(Posted to, among other things, note that LGTM.com exists and is scanning at least some code in Nmap.)

@cldrn
Copy link
Member

cldrn commented Nov 19, 2019

Thanks for the report. I am aware of this and we are working on some patches. We have confirmed the integer overflow in Nping and we will commit a patch soon.

@dmiller-nmap
Copy link

Related commits:

nmap-bot pushed a commit that referenced this issue Dec 25, 2019
@dmiller-nmap
Copy link

Since this issue was filed:

nmap-bot pushed a commit that referenced this issue Dec 29, 2019
nmap-bot pushed a commit that referenced this issue Dec 29, 2019
Our implementation of vsnprintf for systems where it is missing did not
correctly return a negative value on error, instead returning the size
passed in. We got this code from tcpdump/libpcap, and it was wrong
there, too, though their latest master branch has removed it in favor of
requiring a C99 compiler (C99 guarantees vsnprintf).

This should remove a LGTM code analysis finding (See #1834) of
cpp/constant-comparison in Ncat because we were checking for a negative
return from Snprintf, which would never occur.
nmap-bot pushed a commit that referenced this issue Dec 29, 2019
nmap-bot pushed a commit that referenced this issue Dec 29, 2019
nmap-bot pushed a commit that referenced this issue Dec 29, 2019
nmap-bot pushed a commit that referenced this issue Dec 29, 2019
nmap-bot pushed a commit that referenced this issue Dec 30, 2019
I18N installs the `_` function into the builtin namespace, so it looks
like it's unused when it's not. #1834
nmap-bot pushed a commit that referenced this issue Dec 30, 2019
nmap-bot pushed a commit that referenced this issue Jan 1, 2020
nmap-bot pushed a commit that referenced this issue Jan 2, 2020
I added this a long time ago, and decided to check it. In fact,
newstrlen is used to calculate newstrend, and each section of the
template is checked to ensure it does not go past newstrend, so the
intent is met and the length is not exceeded. I still think it could be
written more clearly, but it's good for now. #1834
nmap-bot pushed a commit that referenced this issue Jan 2, 2020
As the FIXME comment had said, looping over every integer up to maxfd is
inefficient, especially if FDs are not continuous. This change has the
added benefit of skipping a call to get_fdinfo(), which also loops over
all the client FDs looking for a particular value. Unlikely to be a huge
performance gain, but the code is cleaner. #1834 - FIXME comment.
@dmiller-nmap
Copy link

Nmap is now at "A+" rating for C/C++ and Python, with only 25 open issues. I have added badges to the README.md that will hopefully serve as a reminder to use LGTM.com to find issues in the future, and I may decide to turn on continuous integration for pull requests, too. For now, I consider this issue resolved.

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

No branches or pull requests

3 participants