[PATCH] RFC: shell-safe version of string_sub()?

Peter Samuelson peter at cadcamlab.org
Sat Oct 21 11:04:07 GMT 2000


  [I wrote]
> > - if instead you use "" to quote, you still have characters you have to
> >   deal with: " ` $ \
> > 
> > This second option is probably easier.

[Jean Francois Micouleau <Jean-Francois.Micouleau at dalalu.fr>]
> yep. Do you still feel like doing it ?

Finally got it done and at least minimally tested.  Patch is against
SAMBA_2_2.  Here's what I did:

- sh_string_sub(): new function, like string_sub() except that it
  quotes substitutions in a shell-safe manner.  In the end, I found
  that the easiest way was actually *not* to use '' or "" but just to
  use \ in front of all suspicious characters.  Suspicious characters
  are found with a lookup table.

- one additional feature of sh_string_sub() which may be controversial:
  any % in the replacement string gets \ immediately after it.  This is
  to take care of the situation where a print job name (%J) happens to
  contain a string such as "%f" -- we do not want to recursively expand
  the job name!  An evil hack makes sure that we only get a single \
  between two "real" characters.

Then...

- printing.c: used sh_string_sub() for the 'print command' parameter.
  This was the original intent of the patch.

- message.c: used sh_string_sub() for the 'message command' parameter.

- reply.c: used sh_string_sub() for 'add script' and 'delete script'
  parameters.  Not really necessary, since Unix usernames should
  already be shell-safe, but this usage is consistent, and harmless.

- nuked alpha_strcpy(), having eliminated the only place it was used.

Note that this patch requires a 'make proto'.  It also requires changes
to an existing smb.conf: you should no longer quote the arguments used
in 'print command' or 'message command'.

Peter

[does anyone have *any idea* how long 'cvs diff' takes?  These days the
public cvs server is s-l-o-w.... (:]



Index: lib/util_str.c
===================================================================
RCS file: /cvsroot/samba/source/lib/util_str.c,v
retrieving revision 1.20
diff -u -r1.20 util_str.c
--- lib/util_str.c	2000/08/29 14:33:39	1.20
+++ lib/util_str.c	2000/10/21 09:41:09
@@ -909,43 +909,6 @@
     return dest;
 }
 
-/*******************************************************************
- Paranoid strcpy into a buffer of given length (includes terminating
- zero. Strips out all but 'a-Z0-9' and replaces with '_'. Deliberately
- does *NOT* check for multibyte characters. Don't change it !
-********************************************************************/
-
-char *alpha_strcpy(char *dest, const char *src, size_t maxlength)
-{
-	size_t len, i;
-
-	if (!dest) {
-		DEBUG(0,("ERROR: NULL dest in alpha_strcpy\n"));
-		return NULL;
-	}
-
-	if (!src) {
-		*dest = 0;
-		return dest;
-	}  
-
-	len = strlen(src);
-	if (len >= maxlength)
-		len = maxlength - 1;
-
-	for(i = 0; i < len; i++) {
-		int val = (src[i] & 0xff);
-		if(isupper(val) ||islower(val) || isdigit(val))
-			dest[i] = src[i];
-		else
-			dest[i] = '_';
-	}
-
-	dest[i] = '\0';
-
-	return dest;
-}
-
 /****************************************************************************
  Like strncpy but always null terminates. Make sure there is room!
  The variable n should always be one less than the available size.
@@ -1186,6 +1149,88 @@
 	string_sub(s, pattern, insert, sizeof(pstring));
 }
 
+/****************************************************************************
+similar to string_sub() but the only mangling done is to make the
+result Bourne-shell-safe.  Use this for constructing shell commands.
+if len==0 then no length check is performed
+****************************************************************************/
+void sh_string_sub(char *s, const char *pattern, const char *insert, size_t len)
+{
+	/* Conservative estimate of Bourne/POSIX shell-safe characters.
+	 * FIXME: are 0x80+ or at least 0xA0+ safe?
+	 */
+	static int is_sh_safe[256] = {
+		0,0,0,0, 0,0,0,0, 0,0,0,0, 0,0,0,0,	/* 0x00             */
+		0,0,0,0, 0,0,0,0, 0,0,0,0, 0,0,0,0,	/* 0x10             */
+		0,0,0,0, 0,0,0,0, 0,0,0,1, 1,1,1,1,	/* 0x20  + , - . /  */
+		1,1,1,1, 1,1,1,1, 1,1,0,0, 0,1,0,0,	/* 0x30  [0-9] =    */
+		1,1,1,1, 1,1,1,1, 1,1,1,1, 1,1,1,1,	/* 0x40  @ [A-O]    */
+		1,1,1,1, 1,1,1,1, 1,1,1,0, 0,0,0,1,	/* 0x50  [P-Z] _    */
+		0,1,1,1, 1,1,1,1, 1,1,1,1, 1,1,1,1,	/* 0x60  [a-o]      */
+		1,1,1,1, 1,1,1,1, 1,1,1,0, 0,0,0,0,	/* 0x70  [p-z]      */
+	};
+
+	char *p;
+	ssize_t ls,lp,li;
+
+	pstring insert2;
+	int ins_init = 0;
+
+	if (!insert || !pattern || !s || !*pattern) return;
+
+	ls = (ssize_t)strlen(s);
+	lp = (ssize_t)strlen(pattern);
+	li = (ssize_t)strlen(insert);
+
+	while (lp <= ls && (p = strstr(s,pattern))) {
+		/*
+		 * escape any non-shell-safe chars with \
+		 * also put \ after % to prevent recursive % subst (hack, hack)
+		 * the insert string is constructed if/when first needed
+		 */
+		if (!ins_init) {
+			ssize_t i,j;
+			int need_esc = 0;
+			for (i=j=0; i<li; i++,j++) {
+				/* don't bother to warn of overflow, as
+				   it will probably be flagged later */
+				if (j > PSTRING_LEN - 2)
+					break;
+				if (insert[i] == '%') {
+					insert2[j++] = '\\';
+					insert2[j] = '%';
+					need_esc = 1;
+					continue;
+				}
+				if (!is_sh_safe[insert[i]])
+					need_esc = 1;
+				if (need_esc) {
+					insert2[j++] = '\\';
+					need_esc = 0;
+				}
+				insert2[j] = insert[i];
+			}
+			/* insert2[j] = '\0'; ...not needed, actually */
+			ins_init = 1;
+			li = j;
+		}
+
+		/* now on to business (cut from all_string_sub()) */
+		if (len && (ls + (li-lp) >= len)) {
+			DEBUG(0,("ERROR: string overflow by %d in sh_string_sub(%.50s, %d)\n",
+				 (int)(ls + (li-lp) - len),
+				 pattern, (int)len));
+			break;
+		}
+		if (li != lp) {
+			memmove(p+li,p+lp,strlen(p+lp)+1);
+		}
+		memcpy(p, insert2, li);
+		s = p + li;
+		ls += (li-lp);
+	}
+
+}
 /****************************************************************************
 similar to string_sub() but allows for any character to be substituted. 
 Use with caution!
Index: printing/printing.c
===================================================================
RCS file: /cvsroot/samba/source/printing/printing.c,v
retrieving revision 1.71.2.5
diff -u -r1.71.2.5 printing.c
--- printing/printing.c	2000/10/11 02:26:50	1.71.2.5
+++ printing/printing.c	2000/10/21 09:43:18
@@ -155,7 +155,7 @@
 	va_start(ap, outfile);
 	while ((arg = va_arg(ap, char *))) {
 		char *value = va_arg(ap,char *);
-		pstring_sub(syscmd, arg, value);
+		sh_string_sub(syscmd, arg, value, sizeof(pstring));
 	}
 	va_end(ap);
   
@@ -850,7 +850,6 @@
 		return False;
 
 	pstrcpy(jobname, pjob->jobname);
-	pstring_sub(jobname, "'", "_");
 
 	/* send it to the system spooler */
 	print_run_command(snum, 
Index: smbd/message.c
===================================================================
RCS file: /cvsroot/samba/source/smbd/message.c,v
retrieving revision 1.17.4.1
diff -u -r1.17.4.1 message.c
--- smbd/message.c	2000/10/10 23:22:49	1.17.4.1
+++ smbd/message.c	2000/10/21 09:46:03
@@ -86,13 +86,10 @@
   /* run the command */
   if (*lp_msg_command())
     {
-      fstring alpha_msgfrom;
-      fstring alpha_msgto;
-
       pstrcpy(s,lp_msg_command());
       pstring_sub(s,"%s",name);
-      pstring_sub(s,"%f",alpha_strcpy(alpha_msgfrom,msgfrom,sizeof(alpha_msgfrom)));
-      pstring_sub(s,"%t",alpha_strcpy(alpha_msgto,msgto,sizeof(alpha_msgto)));
+      sh_string_sub(s, "%f", msgfrom, sizeof(pstring));
+      sh_string_sub(s, "%t", msgto, sizeof(pstring));
       standard_sub_basic(s);
       smbrun(s,NULL,False);
     }
Index: smbd/reply.c
===================================================================
RCS file: /cvsroot/samba/source/smbd/reply.c,v
retrieving revision 1.240.2.8
diff -u -r1.240.2.8 reply.c
--- smbd/reply.c	2000/10/10 18:17:03	1.240.2.8
+++ smbd/reply.c	2000/10/21 10:21:48
@@ -501,7 +501,7 @@
 
   pstrcpy(add_script, lp_adduser_script());
   if (! *add_script) return -1;
-  pstring_sub(add_script, "%u", unix_user);
+  sh_string_sub(add_script, "%u", unix_user, sizeof(pstring));
   ret = smbrun(add_script,NULL,False);
   DEBUG(3,("smb_create_user: Running the command `%s' gave %d\n",add_script,ret));
   return ret;
@@ -518,7 +518,7 @@
 
   pstrcpy(del_script, lp_deluser_script());
   if (! *del_script) return -1;
-  pstring_sub(del_script, "%u", unix_user);
+  sh_string_sub(del_script, "%u", unix_user, sizeof(pstring));
   ret = smbrun(del_script,NULL,False);
   DEBUG(3,("smb_delete_user: Running the command `%s' gave %d\n",del_script,ret));
   return ret;




More information about the samba-technical mailing list