[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