Sfoglia il codice sorgente

check_snmp: Don't thrash memory when using multiple label/unit argument

The memory allocation mixed up number of bytes with number of pointers,
meaning as soon as we'd reach (on 64 bit systems) the second argument,
we'd start writing it outside of our allocated memory.

Normally, this isn't too visible, but as soon as you (again, on my 64
bit system) reach argument number 8, you get a segfault. It is easily
reproducible with:
check_snmp -o '' -l '' -o '' -l '' -o '' -l '' -o '' -l '' \
           -o '' -l '' -o '' -l '' -o '' -l '' -o '' -l ''

This patch allocates the proper amount of memory, to fix the issue.

Signed-off-by: Robin Sonefors <robin.sonefors@op5.com>
Robin Sonefors 13 anni fa
parent
commit
77eba26361
1 ha cambiato i file con 11 aggiunte e 11 eliminazioni
  1. 11 11
      plugins/check_snmp.c

+ 11 - 11
plugins/check_snmp.c

@@ -200,8 +200,8 @@ main (int argc, char **argv)
 	bindtextdomain (PACKAGE, LOCALEDIR);
 	bindtextdomain (PACKAGE, LOCALEDIR);
 	textdomain (PACKAGE);
 	textdomain (PACKAGE);
 
 
-	labels = malloc (labels_size);
-	unitv = malloc (unitv_size);
+	labels = malloc (labels_size * sizeof(*labels));
+	unitv = malloc (unitv_size * sizeof(*unitv));
 	for (i = 0; i < MAX_OIDS; i++)
 	for (i = 0; i < MAX_OIDS; i++)
 		eval_method[i] = CHECK_UNDEF;
 		eval_method[i] = CHECK_UNDEF;
 
 
@@ -768,9 +768,9 @@ process_arguments (int argc, char **argv)
 			break;
 			break;
 		case 'l':									/* label */
 		case 'l':									/* label */
 			nlabels++;
 			nlabels++;
-			if (nlabels >= labels_size) {
+			if (nlabels > labels_size) {
 				labels_size += 8;
 				labels_size += 8;
-				labels = realloc (labels, labels_size);
+				labels = realloc (labels, labels_size * sizeof(*labels));
 				if (labels == NULL)
 				if (labels == NULL)
 					die (STATE_UNKNOWN, _("Could not reallocate labels[%d]"), (int)nlabels);
 					die (STATE_UNKNOWN, _("Could not reallocate labels[%d]"), (int)nlabels);
 			}
 			}
@@ -780,13 +780,13 @@ process_arguments (int argc, char **argv)
 			if (ptr[0] == '\'')
 			if (ptr[0] == '\'')
 				labels[nlabels - 1] = ptr + 1;
 				labels[nlabels - 1] = ptr + 1;
 			while (ptr && (ptr = nextarg (ptr))) {
 			while (ptr && (ptr = nextarg (ptr))) {
-				if (nlabels >= labels_size) {
+				nlabels++;
+				if (nlabels > labels_size) {
 					labels_size += 8;
 					labels_size += 8;
-					labels = realloc (labels, labels_size);
+					labels = realloc (labels, labels_size * sizeof(*labels));
 					if (labels == NULL)
 					if (labels == NULL)
 						die (STATE_UNKNOWN, _("Could not reallocate labels\n"));
 						die (STATE_UNKNOWN, _("Could not reallocate labels\n"));
 				}
 				}
-				nlabels++;
 				ptr = thisarg (ptr);
 				ptr = thisarg (ptr);
 				if (ptr[0] == '\'')
 				if (ptr[0] == '\'')
 					labels[nlabels - 1] = ptr + 1;
 					labels[nlabels - 1] = ptr + 1;
@@ -797,9 +797,9 @@ process_arguments (int argc, char **argv)
 		case 'u':									/* units */
 		case 'u':									/* units */
 			units = optarg;
 			units = optarg;
 			nunits++;
 			nunits++;
-			if (nunits >= unitv_size) {
+			if (nunits > unitv_size) {
 				unitv_size += 8;
 				unitv_size += 8;
-				unitv = realloc (unitv, unitv_size);
+				unitv = realloc (unitv, unitv_size * sizeof(*unitv));
 				if (unitv == NULL)
 				if (unitv == NULL)
 					die (STATE_UNKNOWN, _("Could not reallocate units [%d]\n"), (int)nunits);
 					die (STATE_UNKNOWN, _("Could not reallocate units [%d]\n"), (int)nunits);
 			}
 			}
@@ -809,9 +809,9 @@ process_arguments (int argc, char **argv)
 			if (ptr[0] == '\'')
 			if (ptr[0] == '\'')
 				unitv[nunits - 1] = ptr + 1;
 				unitv[nunits - 1] = ptr + 1;
 			while (ptr && (ptr = nextarg (ptr))) {
 			while (ptr && (ptr = nextarg (ptr))) {
-				if (nunits >= unitv_size) {
+				if (nunits > unitv_size) {
 					unitv_size += 8;
 					unitv_size += 8;
-					unitv = realloc (unitv, unitv_size);
+					unitv = realloc (unitv, unitv_size * sizeof(*unitv));
 					if (units == NULL)
 					if (units == NULL)
 						die (STATE_UNKNOWN, _("Could not realloc() units\n"));
 						die (STATE_UNKNOWN, _("Could not realloc() units\n"));
 				}
 				}