[Spread-users] segmentation fault in spmonitor's option parser

[Neal H. Walfield]

spmonitor is very sloppy about option parsing.  For instance, if you
don't supply a mandatory argument to an option, it crashes.  The
attached patch corrects this.  It also prints out that an option is
unknown when this is the case.  This latter change is arguably a
separate change but it related in the sense that the patch prints out
that a mandatory option is missing, if this is the case, and changes
this error scenario to act similarly.

Relatedly, for reading the port argument, sscanf is used but no
validation check is performed.  In the very least, I think one ought
to check if MY_PORT has the value 0 and error out in this case.  I
have not included a patch to change this behavior.

Neal


2009-11-21  Neal H. Walfield  <neal at gnu.org>

	* monitor.c (Usage): Don't segmentation fault if a mandatory
	option is missing.  If a flag is unknown, say so.

diff -c /home/neal/src/spread-src-4.1.0/daemon/monitor.c\~ /home/neal/src/spread-src-4.1.0/daemon/monitor.c
--- /home/neal/src/spread-src-4.1.0/daemon/monitor.c~	2009-06-18 22:24:17.000000000 +0200
+++ /home/neal/src/spread-src-4.1.0/daemon/monitor.c	2009-11-21 16:59:26.000000000 +0100
@@ -1075,6 +1075,7 @@
 
 static	void	Usage(int argc, char *argv[])
 {
+	char *error = NULL;
 	My_name = 0; /* NULL */
 	My_port = 6543; 
 
@@ -1087,7 +1088,9 @@
 	{
 		argv++;
 
-		if( !strncmp( *argv, "-p", 2 ) )
+		if (argc == 1)
+		        error = "Option missing mandatory argument.";
+		else if( !strncmp( *argv, "-p", 2 ) )
 		{
 			sscanf(argv[1], "%d", &My_port );
 
@@ -1112,8 +1115,12 @@
 			strcpy( Config_file, argv[1] );
 
 			argc--; argv++;
-		}else{
-			Alarm( EXIT, "Usage: spmonitor\n%s\n%s\n%s\n%s\n",
+		}else
+		        error = "Unknown option.";
+
+		if (error) {
+			Alarm( EXIT, "%s\nUsage: spmonitor\n%s\n%s\n%s\n%s\n",
+			        error,
 				"\t[-p <port number>]: specify port number",
 				"\t[-n <proc name>]  : force computer name",
 				"\t[-t <status timeout>]: specify number of seconds between status queries",

Diff finished.  Sat Nov 21 17:00:23 2009