safe string error reporting functions (found a potential buffer overflow

of a pstrcpy into an fstring).
(This used to be commit ac0060443de800fec9042b69b299ff2e9128a31c)

8 files changed, 128 insertions, 74 deletions

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 5ddcc0e650..cca2a6be79 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -597,6 +597,9 @@ void buffer2_to_multistr(char *dest, const BUFFER2 *str, int maxlen);
 
 int cli_set_port(struct cli_state *cli, int port);
 char *cli_errstr(struct cli_state *cli);
+void cli_safe_smb_errstr(struct cli_state *cli, char *msg, size_t len);
+BOOL get_safe_rap_errstr(int rap_error, char *err_msg, size_t msglen);
+void cli_safe_errstr(struct cli_state *cli, char *err_msg, size_t msglen);
 BOOL cli_send_trans(struct cli_state *cli, int trans, 
                            char *name, int pipe_name_len, 
                            int fid, int flags,
@@ -730,6 +733,7 @@ void sort_query_replies(char *data, int n, struct in_addr ip);
 
 /*The following definitions come from  libsmb/nterr.c  */
 
+void get_safe_nt_error_msg(uint32 nt_code, char *msg, size_t len);
 char *get_nt_error_msg(uint32 nt_code);
 
 /*The following definitions come from  libsmb/passchange.c  */
@@ -778,6 +782,8 @@ BOOL nt_decrypt_string2(STRING2 *out, const STRING2 *in, char nt_hash[16]);
 /*The following definitions come from  libsmb/smberr.c  */
 
 char *smb_err_msg(uint8 class, uint32 num);
+BOOL smb_safe_err_msg(uint8 class, uint32 num, char *ret, size_t len);
+BOOL smb_safe_errstr(char *inbuf, char *msg, size_t len);
 char *smb_errstr(char *inbuf);
 
 /*The following definitions come from  locking/locking.c  */
diff --git a/source3/libsmb/clientgen.c b/source3/libsmb/clientgen.c
index bd5d58e4de..cb0f2e5c74 100644
--- a/source3/libsmb/clientgen.c
+++ b/source3/libsmb/clientgen.c
@@ -90,6 +90,26 @@ static BOOL cli_send_smb(struct cli_state *cli, BOOL show)
 	return True;
 }
 
+/******************************************************
+ Return an error message - either an SMB error or a RAP
+ error.
+*******************************************************/
+    
+char *cli_errstr(struct cli_state *cli)
+{   
+	static fstring error_message;
+	cli_safe_errstr(cli, error_message, sizeof(error_message));
+	return error_message;
+}
+
+/****************************************************************************
+  return a description of an SMB error
+****************************************************************************/
+void cli_safe_smb_errstr(struct cli_state *cli, char *msg, size_t len)
+{
+	smb_safe_errstr(cli->inbuf, msg, len);
+}
+
 /*****************************************************
  RAP error codes - a small start but will be extended.
 *******************************************************/
@@ -112,24 +132,32 @@ struct
 };  
 
 /****************************************************************************
-  return a description of an SMB error
+  return a description of a RAP error
 ****************************************************************************/
-static char *cli_smb_errstr(struct cli_state *cli)
+BOOL get_safe_rap_errstr(int rap_error, char *err_msg, size_t msglen)
 {
-	return smb_errstr(cli->inbuf);
+	int i;
+
+	slprintf(err_msg, msglen - 1, "RAP code %d", rap_error);
+
+	for (i = 0; rap_errmap[i].message != NULL; i++)
+	{
+		if (rap_errmap[i].err == rap_error)
+		{
+			safe_strcpy( err_msg, rap_errmap[i].message, msglen);
+			return True;
+		}
+	} 
+	return False;
 }
 
-/******************************************************
- Return an error message - either an SMB error or a RAP
- error.
-*******************************************************/
-    
-char *cli_errstr(struct cli_state *cli)
+/****************************************************************************
+  return a description of an SMB error
+****************************************************************************/
+void cli_safe_errstr(struct cli_state *cli, char *err_msg, size_t msglen)
 {   
-	static fstring error_message;
 	uint8 errclass;
 	uint32 errnum;
-	int i;      
 
 	/*  
 	 * Errors are of three kinds - smb errors,
@@ -142,47 +170,24 @@ char *cli_errstr(struct cli_state *cli)
 
 	if (errclass != 0)
 	{
-		return cli_smb_errstr(cli);
+		cli_safe_smb_errstr(cli, err_msg, msglen);
 	}
-
-	/*
-	 * Was it an NT error ?
-	 */
-
-	if (cli->nt_error)
+	else if (cli->nt_error)
 	{
-		char *nt_msg = get_nt_error_msg(cli->nt_error);
-
-		if (nt_msg == NULL)
-		{
-			slprintf(error_message, sizeof(fstring) - 1, "NT code %d", cli->nt_error);
-		}
-		else
-		{
-			fstrcpy(error_message, nt_msg);
-		}
+		/*
+		 * Was it an NT error ?
+		 */
 
-		return error_message;
+		(void)get_safe_nt_error_msg(cli->nt_error, err_msg, msglen);
 	}
-
-	/*
-	 * Must have been a rap error.
-	 */
-
-	slprintf(error_message, sizeof(error_message) - 1, "code %d", cli->rap_error);
-
-	for (i = 0; rap_errmap[i].message != NULL; i++)
+	else
 	{
-		if (rap_errmap[i].err == cli->rap_error)
-		{
-			fstrcpy( error_message, rap_errmap[i].message);
-			break;
-		}
-	} 
-
-	return error_message;
+		/*
+		 * Must have been a rap error.
+		 */
+		(void)get_safe_rap_errstr(cli->rap_error, err_msg, msglen);
+	}
 }
-
 /****************************************************************************
 setup basics in a outgoing packet
 ****************************************************************************/
diff --git a/source3/libsmb/nterr.c b/source3/libsmb/nterr.c
index b094050a33..9cf1fb8214 100644
--- a/source3/libsmb/nterr.c
+++ b/source3/libsmb/nterr.c
@@ -521,12 +521,11 @@ nt_err_code_struct nt_errs[] =
 /*****************************************************************************
  returns an NT error message.  not amazingly helpful, but better than a number.
  *****************************************************************************/
-char *get_nt_error_msg(uint32 nt_code)
+void get_safe_nt_error_msg(uint32 nt_code, char *msg, size_t len)
 {
-	static pstring msg;
 	int idx = 0;
 
-	snprintf(msg, sizeof(msg), "%08x", nt_code);
+	snprintf(msg, len, "NT code %08x", nt_code);
 
         nt_code &= 0xFFFF;
 
@@ -534,11 +533,19 @@ char *get_nt_error_msg(uint32 nt_code)
 	{
 		if (nt_errs[idx].nt_errcode == nt_code)
 		{
-			pstrcpy(msg, nt_errs[idx].nt_errstr);
-			return msg;
+			safe_strcpy(msg, nt_errs[idx].nt_errstr, len);
+			return;
 		}
 		idx++;
 	}
-	return msg;
 }
 
+/*****************************************************************************
+ returns an NT error message.  not amazingly helpful, but better than a number.
+ *****************************************************************************/
+char *get_nt_error_msg(uint32 nt_code)
+{
+	static pstring msg;
+	get_safe_nt_error_msg(nt_code, msg, sizeof(msg));
+	return msg;
+}
diff --git a/source3/libsmb/smberr.c b/source3/libsmb/smberr.c
index 85827dde28..228eee5892 100644
--- a/source3/libsmb/smberr.c
+++ b/source3/libsmb/smberr.c
@@ -143,13 +143,19 @@ struct
   {0xFF,"ERRCMD",NULL},
   {-1,NULL,NULL}};
 
+char *smb_err_msg(uint8 class, uint32 num)
+{
+	static pstring ret;
+	smb_safe_err_msg(class, num, ret, sizeof(ret));
+	return ret;
+}
+	
 
 /****************************************************************************
 return a SMB error string from a SMB buffer
 ****************************************************************************/
-char *smb_err_msg(uint8 class, uint32 num)
+BOOL smb_safe_err_msg(uint8 class, uint32 num, char *ret, size_t len)
 {
-	static pstring ret;
 	int i,j;
 
 	for (i=0;err_classes[i].class;i++)
@@ -165,29 +171,42 @@ char *smb_err_msg(uint8 class, uint32 num)
 					{
 						if (DEBUGLEVEL > 0)
 						{
-							slprintf(ret, sizeof(ret) - 1, "%s - %s (%s)",err_classes[i].class,
+							slprintf(ret, len - 1, "%s - %s (%s)",err_classes[i].class,
 							err[j].name,err[j].message);
 						}
 						else
 						{
-							slprintf(ret, sizeof(ret) - 1, "%s - %s",err_classes[i].class,err[j].name);
+							slprintf(ret, len - 1, "%s - %s",err_classes[i].class,err[j].name);
 						}
-						return ret;
+						return True;
 					}
 				}
 			}
-			slprintf(ret, sizeof(ret) - 1, "%s - %d",err_classes[i].class, num);
-			return ret;
+			slprintf(ret, len - 1, "%s - %d",err_classes[i].class, num);
+			return True;
 		}
 
 	}
-	slprintf(ret, sizeof(ret) - 1, "Error: Unknown error (%d,%d)",class,num);
-	return(ret);
+
+	slprintf(ret, len - 1, "Error: Unknown error (%d,%d)",class,num);
+	return False;
 }
+
+/****************************************************************************
+return a SMB error string from a SMB buffer
+****************************************************************************/
+BOOL smb_safe_errstr(char *inbuf, char *msg, size_t len)
+{
+	return smb_safe_err_msg(CVAL(inbuf,smb_rcls), SVAL(inbuf,smb_err),
+				msg, len);
+}
+
 /****************************************************************************
 return a SMB error string from a SMB buffer
 ****************************************************************************/
 char *smb_errstr(char *inbuf)
 {
-	return smb_err_msg(CVAL(inbuf,smb_rcls), SVAL(inbuf,smb_err));
+	static fstring errmsg;
+	(void)smb_safe_errstr(inbuf, errmsg, sizeof(errmsg));
+	return errmsg;
 }
diff --git a/source3/rpc_client/cli_netlogon.c b/source3/rpc_client/cli_netlogon.c
index e9a8582d10..d385011bac 100644
--- a/source3/rpc_client/cli_netlogon.c
+++ b/source3/rpc_client/cli_netlogon.c
@@ -571,8 +571,10 @@ to ourselves.\n", remote_machine));
   }
 
   if (!cli_connect(&cli, remote_machine, &cli.dest_ip)) {
+	fstring errstr;
+    cli_safe_errstr(&cli, errstr, sizeof(errstr));
     DEBUG(0,("modify_trust_password: unable to connect to SMB server on \
-machine %s. Error was : %s.\n", remote_machine, cli_errstr(&cli) ));
+machine %s. Error was : %s.\n", remote_machine, errstr  ));
     return False;
   }
     
@@ -582,8 +584,10 @@ machine %s. Error was : %s.\n", remote_machine, cli_errstr(&cli) ));
 
 	if (!cli_session_request(&cli, &calling, &called))
 	{
+	fstring errstr;
+    cli_safe_errstr(&cli, errstr, sizeof(errstr));
     DEBUG(0,("modify_trust_password: machine %s rejected the session setup. \
-Error was : %s.\n", remote_machine, cli_errstr(&cli) ));
+Error was : %s.\n", remote_machine, errstr ));
     cli_shutdown(&cli);
     return False;
   }
@@ -591,8 +595,10 @@ Error was : %s.\n", remote_machine, cli_errstr(&cli) ));
   cli.protocol = PROTOCOL_NT1;
     
   if (!cli_negprot(&cli)) {
+	fstring errstr;
+    cli_safe_errstr(&cli, errstr, sizeof(errstr));
     DEBUG(0,("modify_trust_password: machine %s rejected the negotiate protocol. \
-Error was : %s.\n", remote_machine, cli_errstr(&cli) ));
+Error was : %s.\n", remote_machine, errstr ));
     cli_shutdown(&cli);
     return False;
   }
@@ -608,8 +614,10 @@ Error was : %s.\n", remote_machine, cli_errstr(&cli) ));
    */
     
   if (!cli_session_setup(&cli, "", "", 0, "", 0, "")) {
+	fstring errstr;
+    cli_safe_errstr(&cli, errstr, sizeof(errstr));
     DEBUG(0,("modify_trust_password: machine %s rejected the session setup. \
-Error was : %s.\n", remote_machine, cli_errstr(&cli) ));
+Error was : %s.\n", remote_machine, errstr  ));
     cli_shutdown(&cli);
     return False;
   }
@@ -622,8 +630,10 @@ Error was : %s.\n", remote_machine, cli_errstr(&cli) ));
   }
     
   if (!cli_send_tconX(&cli, "IPC$", "IPC", "", 1)) {
+	fstring errstr;
+    cli_safe_errstr(&cli, errstr, sizeof(errstr));
     DEBUG(0,("modify_trust_password: machine %s rejected the tconX on the IPC$ share. \
-Error was : %s.\n", remote_machine, cli_errstr(&cli) ));
+Error was : %s.\n", remote_machine, errstr ));
     cli_shutdown(&cli);
     return False;
   }
@@ -634,8 +644,10 @@ Error was : %s.\n", remote_machine, cli_errstr(&cli) ));
    */
     
   if(cli_nt_session_open(&cli, PIPE_NETLOGON, &nt_pipe_fnum) == False) {
+	fstring errstr;
+    cli_safe_errstr(&cli, errstr, sizeof(errstr));
     DEBUG(0,("modify_trust_password: unable to open the domain client session to \
-machine %s. Error was : %s.\n", remote_machine, cli_errstr(&cli)));
+machine %s. Error was : %s.\n", remote_machine, errstr ));
     cli_nt_session_close(&cli, nt_pipe_fnum);
     cli_ulogoff(&cli);
     cli_shutdown(&cli);
@@ -644,8 +656,10 @@ machine %s. Error was : %s.\n", remote_machine, cli_errstr(&cli)));
   
   if(cli_nt_setup_creds(&cli, nt_pipe_fnum,
      cli.mach_acct, orig_trust_passwd_hash, sec_chan) == False) {
+	fstring errstr;
+    cli_safe_errstr(&cli, errstr, sizeof(errstr));
     DEBUG(0,("modify_trust_password: unable to setup the PDC credentials to machine \
-%s. Error was : %s.\n", remote_machine, cli_errstr(&cli)));
+%s. Error was : %s.\n", remote_machine, errstr ));
     cli_nt_session_close(&cli, nt_pipe_fnum);
     cli_ulogoff(&cli);
     cli_shutdown(&cli);
@@ -653,9 +667,11 @@ machine %s. Error was : %s.\n", remote_machine, cli_errstr(&cli)));
   } 
 
   if( cli_nt_srv_pwset( &cli, nt_pipe_fnum, new_trust_passwd_hash, sec_chan ) == False) {
+	fstring errstr;
+    cli_safe_errstr(&cli, errstr, sizeof(errstr));
     DEBUG(0,("modify_trust_password: unable to change password for machine %s in domain \
 %s to Domain controller %s. Error was %s.\n", global_myname, domain, remote_machine, 
-                            cli_errstr(&cli)));
+                            errstr ));
     cli_nt_session_close(&cli, nt_pipe_fnum);
     cli_ulogoff(&cli);
     cli_shutdown(&cli);
diff --git a/source3/rpc_client/cli_svcctl.c b/source3/rpc_client/cli_svcctl.c
index ebf8c15dc4..5e8a3952a5 100644
--- a/source3/rpc_client/cli_svcctl.c
+++ b/source3/rpc_client/cli_svcctl.c
@@ -199,8 +199,11 @@ BOOL svc_enum_svcs(struct cli_state *cli, uint16 fnum,
 
 		if (p && r_o.dos_status != 0)
 		{
+			fstring errmsg;
+			smb_safe_err_msg(ERRDOS, r_o.dos_status,
+				errmsg, sizeof(errmsg));
 			/* report error code */
-			DEBUG(0,("SVC_ENUM_SVCS_STATUS: %s\n", smb_err_msg(ERRDOS, r_o.dos_status)));
+			DEBUG(0,("SVC_ENUM_SVCS_STATUS: %s\n", errmsg));
 			p = r_o.dos_status == ERRmoredata;
 		}
 
diff --git a/source3/rpcclient/rpcclient.c b/source3/rpcclient/rpcclient.c
index 9bfb80aa3d..259c10d11f 100644
--- a/source3/rpcclient/rpcclient.c
+++ b/source3/rpcclient/rpcclient.c
@@ -39,8 +39,6 @@ extern pstring user_socket_options;
 extern int DEBUGLEVEL;
 
 
-extern file_info def_finfo;
-
 #define CNV_LANG(s) dos2unix_format(s,False)
 #define CNV_INPUT(s) unix2dos_format(s,True)
 
diff --git a/source3/utils/torture.c b/source3/utils/torture.c
index 2de9681031..3c872ea672 100644
--- a/source3/utils/torture.c
+++ b/source3/utils/torture.c
@@ -1223,9 +1223,9 @@ static void create_procs(int nprocs, int numops, void (*fn)(int ))
 
 	create_procs(nprocs, numops, run_randomipc_nowait);
 /*
+	create_procs(nprocs, numops, run_randomipc);
 
 	create_procs(nprocs, numops, run_connection);
-	create_procs(nprocs, numops, run_randomipc);
 
 	run_fdpasstest();
 	run_locktest1();