[Patch] Runtime interaction bug with sudo

[Jay Bosamiya <jaybosamiya () gmail com>]

The problem with not being able to grab keystrokes when running nmap 
through sudo or through a
script lies in the fact that a comparison of tcgetpgrp(tty_fd) and 
getpid() is used to ascertain
whether to stop keystroke grabbing or to continue. (This is in file 
nmap_tty.cc)

Changing getpid() to getpgrp() does not affect normal execution of nmap 
at all (AFAIK) and also
lets keystrokes be grabbed when in sudo or through a script. However, if 
a SIGINT is sent
(through Ctrl+C) then the control is returned to the terminal where 
keyboard/display does not work
correctly. It tends to work without any echoing of input to the 
terminal. So, after the Ctrl+C,
the terminal is basically unusable (in any practical sense).

In order to rectify this, the SIGINT must be handled and be converted 
into an exit() so that
tty_done() is called and everything exits nicely. In order to do this, I 
created a function
and set it as the handler for SIGINT. This function calls exit() with 
EXIT_FAILURE as parameter
(since I felt that the it is an unnatural exit, but this can be changed 
to EXIT_SUCCESS if that
seems more appropriate).

According to the signal man page, sigaction() is more preferred and 
signal() must be avoided.
I will look into this and change the code to sigaction() if necessary.

Looking at [1] and the discussion before and after it, it seems like 
Patrick Donnelly and David Fifield
have already looked into signals and the possibility of installing a 
signal handler. And according to
[2], David says that it'd be better not to do anything complicated in a 
signal handler because of subtle
security risks. However, I feel that a simple exit() in a signal handler 
for SIGINT would be a very
simple way to work things out correctly and not add security risks. 
Correct me if I am wrong.

So, putting all this together, I tested it on Ubuntu and found that 
things seem to work fine (as far as
I can see) but I'd like some feedback.

Attached is the patch.

Cheers,
Jay

References:
[1] http://seclists.org/nmap-dev/2012/q2/73
[2] http://seclists.org/nmap-dev/2012/q2/86

Attachment: keyStrokeGrabbing.patch
Description:

_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/