Merge pull request #8 from nibroc/cmdline-overflow

Fixed buffer overflow for cmdline strings of length >= 80, closes #7

inode2prog.cpp

@@ -39,6 +39,12 @@
 
 extern bool bughuntmode;
 
+// Not sure, but assuming there's no more PID's than go into 64 unsigned bits..
+const int MAX_PID_LENGTH = 20;
+
+// Max length of filenames in /proc/<pid>/fd/*. These are numeric, so 10 digits seems like a safe assumption.
+const int MAX_FDLINK = 10;
+
 /* maps from inode to program-struct */
 std::map <unsigned long, prg_node *> inodeproc;
 
@@ -73,46 +79,59 @@ int str2int (const char * ptr) {
 	return retval;
 }
 
-// Not sure, but assuming there's no more PID's than go into 64 unsigned bits..
+static std::string read_file (int fd) {
-#define MAX_PID_LENGTH 20
+	char buf[255];
+	std::string content;
 
-char * getprogname (pid_t pid) {
+	for (int length; (length = read(fd, buf, sizeof(buf))) > 0;) {
-	int bufsize = 80;
+		if (length < 0) {
-	char buffer [bufsize];
+			std::fprintf(stderr, "Error reading file: %s\n", std::strerror(errno));
-
+			std::exit(34);
-	int maxfilenamelen = 14 + MAX_PID_LENGTH + 1; 
+		}
-	char filename[maxfilenamelen];
+		content.append(buf, length);
-
-	snprintf (filename, maxfilenamelen, "/proc/%d/cmdline", pid);
-	int fd = open(filename, O_RDONLY);
-	if (fd < 0) {
-		fprintf (stderr, "Error opening %s: %s\n", filename, strerror(errno));
-		exit(3);
-		return NULL;
 	}
-	int length = read (fd, buffer, bufsize);
-	if (close (fd)) {
-		std::cout << "Error closing file: " << strerror(errno) << std::endl;
-		exit(34);
-	}
-	if (length < bufsize - 1)
-		buffer[length]='\0';
 
-	return strdup(buffer);
+	return content;
 }
 
-void setnode (unsigned long inode, pid_t pid)
+static std::string read_file (const char* filepath) {
-{	
+	int fd = open(filepath, O_RDONLY);
+
+	if (fd < 0) {
+		std::fprintf(stderr, "Error opening %s: %s\n", filepath, std::strerror(errno));
+		std::exit(3);
+		return NULL;
+	}
+
+	std::string contents = read_file(fd);
+
+	if (close(fd)) {
+		std::fprintf(stderr, "Error opening %s: %s\n", filepath, std::strerror(errno));
+		std::exit(34);
+	}
+
+	return contents;
+}
+
+std::string getprogname (pid_t pid) {
+	const int maxfilenamelen = 14 + MAX_PID_LENGTH + 1;
+	char filename[maxfilenamelen];
+
+	std::snprintf(filename, maxfilenamelen, "/proc/%d/cmdline", pid);
+	return read_file(filename);
+}
+
+void setnode (unsigned long inode, pid_t pid) {
 	prg_node * current_value = inodeproc[inode];
 
 	if (current_value == NULL || current_value->pid != pid) {
-		prg_node * newnode = (prg_node *) malloc (sizeof (struct prg_node));
+		prg_node * newnode = new prg_node;
 		newnode->inode = inode;
 		newnode->pid   = pid;
 		newnode->name  = getprogname(pid);
 
 		inodeproc[inode] = newnode;
-		free(current_value);
+		delete current_value;
 	}
 }
 
@@ -122,9 +141,6 @@ void get_info_by_linkname (const char * pid, const char * linkname) {
 	}
 }
 
-// Max length of filenames in /proc/<pid>/fd/*. These are numeric, so 10 digits seems like a safe assumption.
-#define MAX_FDLINK 10
-
 /* updates the `inodeproc' inode-to-prg_node
  * for all inodes belonging to this PID
  * (/proc/pid/fd/42)

inode2prog.h

@@ -30,7 +30,7 @@
 struct prg_node {
     long inode;
     pid_t pid;
-    char * name;
+    std::string name;
 };
 
 struct prg_node * findPID (unsigned long inode);

process.cpp

@@ -165,7 +165,7 @@ Process * getProcess (unsigned long inode, const char * devicename)
 	if (proc != NULL)
 		return proc;
 
-	Process * newproc = new Process (inode, devicename, node->name);
+	Process * newproc = new Process (inode, devicename, node->name.c_str());
 	newproc->pid = node->pid;
 
 	char procdir [100];