[PATCH] RFC: shell-safe version of string_sub()?
Peter Samuelson
peter at cadcamlab.org
Sun Nov 5 18:54:58 GMT 2000
[I wrote, some time ago]
> - 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.
That patch had a subtle bug that I just discovered this morning. If
you make a substitution and the replacement is zero-length, the shell
will never see that argument separately. The fix is easy: check for
this case and insert ''.
The incremental patch:
--- source/lib/util_str.c~ Sat Oct 21 05:33:33 2000
+++ source/lib/util_str.c Sun Nov 5 11:43:40 2000
@@ -1210,6 +1210,12 @@
}
insert2[j] = insert[i];
}
+ /* special case: zero-length string should become '' */
+ if (!li)
+ {
+ pstrcpy (insert2, "''");
+ j = 2;
+ }
/* insert2[j] = '\0'; ----- not needed, actually */
ins_init = 1;
li = j;
The full patch:
Index: source/include/proto.h
===================================================================
RCS file: /cvsroot/samba/source/include/proto.h,v
retrieving revision 1.900.2.26
diff -u -r1.900.2.26 proto.h
--- source/include/proto.h 2000/10/28 19:33:28 1.900.2.26
+++ source/include/proto.h 2000/10/30 05:35:46
@@ -522,7 +522,6 @@
BOOL str_is_all(const char *s,char c);
char *safe_strcpy(char *dest,const char *src, size_t maxlength);
char *safe_strcat(char *dest, const char *src, size_t maxlength);
-char *alpha_strcpy(char *dest, const char *src, size_t maxlength);
char *StrnCpy(char *dest,const char *src,size_t n);
char *strncpyn(char *dest, const char *src,size_t n, char c);
size_t strhex_to_str(char *p, size_t len, const char *strhex);
@@ -532,6 +531,7 @@
void string_sub(char *s,const char *pattern,const char *insert, size_t len);
void fstring_sub(char *s,const char *pattern,const char *insert);
void pstring_sub(char *s,const char *pattern,const char *insert);
+void sh_string_sub(char *s, const char *pattern, const char *insert, size_t len);
void all_string_sub(char *s,const char *pattern,const char *insert, size_t len);
void split_at_last_component(char *path, char *front, char sep, char *back);
char *octal_string(int i);
Index: source/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
--- source/lib/util_str.c 2000/08/29 14:33:39 1.20
+++ source/lib/util_str.c 2000/10/30 05:35:46
@@ -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,94 @@
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 substitution 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];
+ }
+ /* special case: zero-length string should become '' */
+ if (!li)
+ {
+ pstrcpy (insert2, "''");
+ j = 2;
+ }
+ /* 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: source/printing/printing.c
===================================================================
RCS file: /cvsroot/samba/source/printing/printing.c,v
retrieving revision 1.71.2.6
diff -u -r1.71.2.6 printing.c
--- source/printing/printing.c 2000/10/21 01:30:41 1.71.2.6
+++ source/printing/printing.c 2000/10/30 05:35:46
@@ -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);
@@ -848,7 +848,6 @@
return False;
pstrcpy(jobname, pjob->jobname);
- pstring_sub(jobname, "'", "_");
/* send it to the system spooler */
print_run_command(snum,
Index: source/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
--- source/smbd/message.c 2000/10/10 23:22:49 1.17.4.1
+++ source/smbd/message.c 2000/10/30 05:35:47
@@ -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: source/smbd/reply.c
===================================================================
RCS file: /cvsroot/samba/source/smbd/reply.c,v
retrieving revision 1.240.2.9
diff -u -r1.240.2.9 reply.c
--- source/smbd/reply.c 2000/10/25 01:09:30 1.240.2.9
+++ source/smbd/reply.c 2000/10/30 05:35:49
@@ -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