[torquedev] patch: bounds checking on PBatchReqType[] accesses

[Sergio Gelato]

pbs_server references the array PBatchReqType[] when generating error
messages about requests received from the network.

Unfortunately, it doesn't check that the request type lies within the 
array bounds. A malicious client could therefore easily cause an
almost arbitrary portion of the pbs_server address space to be 
copied to log_buffer, possibly overflowing it (and/or causing a
segmentation fault). 

The attached patch is intended to close this loophole.
-------------- next part --------------
--- torque-2.1.6/src/include/pbs_error.h	2006-09-26 00:52:25.000000000 +0200
+++ torque-2.1.6/src/include/pbs_error.h	2007-02-13 11:35:15.406295509 +0100
@@ -191,6 +191,7 @@
   };
 
 extern char *pbse_to_txt(int);
+extern const char *reqtype_to_txt(int);
 
 extern int pbs_errno;
 
--- torque-2.1.6/src/lib/Liblog/pbs_messages.c	2006-09-26 00:52:26.000000000 +0200
+++ torque-2.1.6/src/lib/Liblog/pbs_messages.c	2007-02-13 11:38:46.014050596 +0100
@@ -172,7 +172,7 @@
 
 /* sync w/enum PBatchReqTypeEnum in libpbs.h */
 
-const char *PBatchReqType[] = {
+static const char *PBatchReqType[] = {
   "Connect",
   "QueueJob",
   "JobCred",
@@ -235,6 +235,8 @@
   "Disconnect",
   NULL };
 
+#define NPBatchReqType (sizeof(PBatchReqType)/sizeof(PBatchReqType[0]))
+
 /*
  * This next set of messages are returned to the client on an error.
  * They may also be logged.
@@ -442,4 +444,17 @@
   }  /* END pbse_to_txt() */
 
 
+/*
+ * rqtype_to_txt()	- Return the printable name of a request type
+ */
+
+const char *reqtype_to_txt(
 
+  int reqtype)
+
+  {
+  if (reqtype >= 0 && reqtype < NPBatchReqType && PBatchReqType[reqtype])
+    return(PBatchReqType[reqtype]);
+  else
+    return("NONE");
+  }  /* END reqtype_to_txt() */
--- torque-2.1.6/src/server/dis_read.c	2006-07-31 22:11:53.000000000 +0200
+++ torque-2.1.6/src/server/dis_read.c	2007-02-13 11:38:22.145378936 +0100
@@ -105,7 +105,6 @@
 /* External Global Data */
 
 extern char	*msg_nosupport;
-extern char     *PBatchReqType[];
 
 extern int       LOGLEVEL;
 
@@ -170,7 +169,7 @@
   if (LOGLEVEL >= 5)
     {
     sprintf(log_buffer,"decoding command %s from %s",
-      PBatchReqType[request->rq_type],
+      reqtype_to_txt(request->rq_type),
       request->rq_user);
 
     LOG_EVENT(
@@ -347,7 +346,7 @@
 
       sprintf(log_buffer,"%s: %s from %s", 
         msg_nosupport, 
-        PBatchReqType[request->rq_type], 
+        reqtype_to_txt(request->rq_type), 
         request->rq_user);
 
       LOG_EVENT(
@@ -370,7 +369,7 @@
       sprintf(log_buffer,"req extension bad, dis error %d (%s), type=%s", 
         rc,
         dis_emsg[rc],
-        PBatchReqType[request->rq_type]);
+        reqtype_to_txt(request->rq_type));
 
       LOG_EVENT(
         PBSEVENT_DEBUG, 
@@ -386,7 +385,7 @@
     sprintf(log_buffer,"req body bad, dis error %d (%s), type=%s",
       rc, 
       dis_emsg[rc],
-      PBatchReqType[request->rq_type]);
+      reqtype_to_txt(request->rq_type));
 
     LOG_EVENT(PBSEVENT_DEBUG,PBS_EVENTCLASS_REQUEST,"?",log_buffer);
 
--- torque-2.1.6/src/server/process_request.c	2006-10-03 04:28:01.000000000 +0200
+++ torque-2.1.6/src/server/process_request.c	2007-02-13 11:37:25.294396722 +0100
@@ -129,8 +129,6 @@
 extern char  *msg_reqbadhost;
 extern char  *msg_request;
 
-extern const char *PBatchReqType[];
-
 extern int LOGLEVEL;
 
 /* private functions local to this file */
@@ -299,7 +297,7 @@
   sprintf(
     log_buffer,
     msg_request,
-    PBatchReqType[request->rq_type],
+    reqtype_to_txt(request->rq_type),
     request->rq_user,
     request->rq_host,
     sfds);
@@ -429,7 +427,7 @@
   if (LOGLEVEL >= 6)
     {
     sprintf(log_buffer,"request type %s from host %s received",
-      PBatchReqType[request->rq_type],
+      reqtype_to_txt(request->rq_type),
       request->rq_host);
 
     log_record(
@@ -442,7 +440,7 @@
   if (!tfind(svr_conn[sfds].cn_addr,&okclients)) 
     {
     sprintf(log_buffer,"request type %s from host %s rejected (host not authorized)",
-      PBatchReqType[request->rq_type],
+      reqtype_to_txt(request->rq_type),
       request->rq_host);
 
     log_record(
@@ -461,7 +459,7 @@
   if (LOGLEVEL >= 3)
     {
     sprintf(log_buffer,"request type %s from host %s allowed",
-      PBatchReqType[request->rq_type],
+      reqtype_to_txt(request->rq_type),
       request->rq_host);
 
     log_record(
@@ -472,7 +470,7 @@
     }
 
   MOMLastRecvFromServerTime = time_now;
-  strcpy(MOMLastRecvFromServerCmd,PBatchReqType[request->rq_type]);
+  strcpy(MOMLastRecvFromServerCmd,reqtype_to_txt(request->rq_type));
   }    /* END BLOCK */
 		
   request->rq_fromsvr = 1;
@@ -518,7 +516,7 @@
   if (LOGLEVEL >= 5)
     {
     sprintf(log_buffer,"dispatching request %s on sd=%d",
-      PBatchReqType[request->rq_type],
+      reqtype_to_txt(request->rq_type),
       sfds);
 
     log_record(
--- torque-2.1.6/src/server/reply_send.c	2006-10-03 04:28:00.000000000 +0200
+++ torque-2.1.6/src/server/reply_send.c	2007-02-13 11:35:06.190649738 +0100
@@ -120,7 +120,6 @@
 #endif	/* PBS_MOM */
 
 extern struct pbs_err_to_txt pbs_err_to_txt[];
-extern const char *PBatchReqType[];
 
 #define ERR_MSG_SIZE 127
 
@@ -402,7 +401,7 @@
     code, 
     msgbuf,
     aux, 
-    PBatchReqType[preq->rq_type], 
+    reqtype_to_txt(preq->rq_type), 
     preq->rq_user, 
     preq->rq_host);