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
Comments
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. |
Since this issue was filed:
|
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.
I18N installs the `_` function into the builtin namespace, so it looks like it's unused when it's not. #1834
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
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.
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. |
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.)
The text was updated successfully, but these errors were encountered: