Re: Pine 4.33 (at least) URL handler allows embedded commands.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> > Problem:		URL handler allows embedded commands.
> > 			May allow email viruses of the Outlook kind.
>
> >   http://address/'&/some/program${IFS}with${IFS}arguments&'
>
> Isn't that old news? http://www.securityfocus.com/bid/810
>
> I *can* be wrong, but it looks like it is the same problem...

SuSE pine packages contain a patch that makes pine use environment
variables to pass on the URL to the viewer. The patch is attached - I'm
not sure who made it, but it looks like from Olaf Kirch.

Roman.
-- 
 -                                                                      -
| Roman Drahtmüller      <draht@suse.de> // "You don't need eyes to see, |
  SuSE GmbH - Security           Phone: //             you need vision!"
| Nürnberg, Germany     +49-911-740530 //           Maxi Jazz, Faithless |
 -                                                                      -
--- pine/mailview.c.orig	Thu Oct 12 21:33:32 2000
+++ pine/mailview.c	Fri Oct 27 10:04:58 2000
@@ -3738,124 +3738,46 @@
 #define	URL_MAX_LAUNCH	(2 * MAILTMPLEN)
 
     if(handle->h.url.tool){
-	char	*toolp, *cmdp, *p, *q, cmd[URL_MAX_LAUNCH + 1];
-	char    *left_double_quote, *right_double_quote;
-	int	 mode, len, hlen, quotable = 0, copied = 0, double_quoted = 0;
+	char	*toolp, *cmdp, *endp, cmd[URL_MAX_LAUNCH + 1];
+	int	 mode, len, copied = 0;
 	PIPE_S *syspipe;
 
 	if((len = strlen(toolp = handle->h.url.tool)) > URL_MAX_LAUNCH)
 	  return(url_launch_too_long(rv));
 	  
-	hlen	 = strlen(handle->h.url.path);
-
 	/*
-	 * Figure out if we need to quote the URL. If there are shell
-	 * metacharacters in it we want to quote it, because we don't want
-	 * the shell to interpret them. However, if the user has already
-	 * quoted the URL in the command definition we don't want to quote
-	 * again. So, we try to see if there are a pair of unescaped
-	 * quotes surrounding _URL_ in the cmd.
-	 * If we quote when we shouldn't have, it'll cause it not to work.
-	 * If we don't quote when we should have, it's a possible security
-	 * problem (and it still won't work).
-	 *
-	 * In bash and ksh $( executes a command, so we use single quotes
-	 * instead of double quotes to do our quoting. If configured command
-	 * is double-quoted we change that to single quotes.
+	 * Rather than trying to be smart about quoting and
+	 * meta-characters, just stuff the URL into an environment
+	 * variable and make the handler use it.
 	 */
-#ifdef	_WINDOWS
-	if(*toolp == '*' || (*toolp == '\"' && *(toolp+1) == '*'))
-	  quotable = 0;		/* never quote */
-	else
-#endif
-	if(strpbrk(handle->h.url.path, "&*;<>?[|~$") != NULL){  /* specials? */
-	    if((p = strstr(toolp, "_URL_")) != NULL){  /* explicit arg? */
-		int in_quote = 0;
-
-		/* see whether or not it is already quoted */
-
-	        quotable = 1;
-
-		for(q = toolp; q < p; q++)
-		  if(*q == '\'' && (q == toolp || q[-1] != '\\'))
-		    in_quote = 1 - in_quote;
-		
-		if(in_quote){
-		    for(q = p+5; *q; q++)
-		      if(*q == '\'' && q[-1] != '\\'){
-			  /* already single quoted, leave it alone */
-			  quotable = 0;
-			  break;
-		      }
-		}
-
-		if(quotable){
-		    in_quote = 0;
-		    for(q = toolp; q < p; q++)
-		      if(*q == '\"' && (q == toolp || q[-1] != '\\')){
-			  in_quote = 1 - in_quote;
-			  if(in_quote)
-			    left_double_quote = q;
-		      }
-		    
-		    if(in_quote){
-			for(q = p+5; *q; q++)
-			  if(*q == '\"' && q[-1] != '\\'){
-			      /* we'll replace double quotes with singles */
-			      double_quoted = 1;
-			      right_double_quote = q;
-			      break;
-			  }
-		    }
-		}
-	    }
-	    else
-	      quotable = 1;
-	}
-	else
-	  quotable = 0;
+	setenv("URL", handle->h.url.path, 1);
+#define _URL_EXPANSION	"\"$URL\""
 
 	/* Build the command */
 	cmdp = cmd;
-	while(1)
-	  if((!*toolp && !copied)
-	     || (*toolp == '_' && !strncmp(toolp + 1, "URL_", 4))){
+	endp = cmd + sizeof(cmd) - 1;
+	do {
+	  if (cmdp + 1 > endp)
+	      return(url_launch_too_long(rv));
 
+	  if (!*toolp && !copied) {
 	      /* implicit _URL_ at end */
-	      if(!*toolp){
-		  *cmdp++ = ' ';
-		  len++;
-	      }
-
-	      /* add single quotes */
-	      if(quotable && !double_quoted){
-		  *cmdp++ = '\'';
-		  len += 2;
-	      }
+	      *endp++ = ' ';
+	      toolp = "_URL_";
+	  }
+
+	  if (strncmp(toolp, "_URL_", 5) != 0) {
+	      *cmdp++ = *toolp++;
+	  } else {
+	      toolp += 5; /* length of _URL_ */
 
-	      if((len += hlen) > URL_MAX_LAUNCH)
+	      if (cmdp + sizeof(_URL_EXPANSION) - 1 > endp)
 		return(url_launch_too_long(rv));
 
+	      sstrcpy(&cmdp, _URL_EXPANSION);
 	      copied = 1;
-	      sstrcpy(&cmdp, handle->h.url.path);
-	      if(quotable && !double_quoted){
-		  *cmdp++ = '\'';
-		  *cmdp = '\0';
-	      }
-
-	      if(*toolp)
-		toolp += 5;		/* length of "_URL_" */
-	  }
-	  else{
-	      /* replace double quotes with single quotes */
-	      if(double_quoted &&
-		 (toolp == left_double_quote || toolp == right_double_quote)){
-		  *cmdp++ = '\'';
-		  toolp++;
-	      }
-	      else if(!(*cmdp++ = *toolp++))
-		break;
 	  }
+	} while (*toolp);
 	
 	mode = PIPE_RESET | PIPE_USER ;
 	if(syspipe = open_system_pipe(cmd, NULL, NULL, mode, 0)){

[Index of Archives]     [Linux Security]     [Netfilter]     [PHP]     [Yosemite News]     [Linux Kernel]

  Powered by Linux