Proposed patch for psql backtick safety

Started by Tom Lanealmost 22 years ago1 messagespatches

tgl@sss.pgh.pa.us

almost 22 years ago

Attached is a proposed patch for 8.0 to address the concerns raised
in this thread:
http://archives.postgresql.org/pgsql-bugs/2004-12/msg00189.php

The patch extends the psql lexer to have an additional argument mode
OT_VERBATIM, which is like OT_NORMAL except that backtick and variable
substitutions don't happen. (Variable substitution is not dangerous
AFAICS, but I thought that this definition might have other uses later.)

Then, it makes two changes in post-backslash-command cleanup:

1. Following a successful command, any "extra" arguments are eaten in
OT_VERBATIM instead of OT_NORMAL mode. For example, since \t takes
no arguments:

regression=# \t `echo bozo`
Showing only tuples.
\t: extra argument "`echo" ignored
\t: extra argument "bozo`" ignored
regression=#

whereas formerly the backtick command would be evaluated:

regression=# \t `echo bozo`
Showing only tuples.
\t: extra argument "bozo" ignored
regression=#

2. Following an unrecognized or unsuccessful command, the rest of the
line is eaten (in OT_WHOLE_LINE mode, which doesn't do backticks or
variables either). For example,

regression=# \N `touch bozo`
Invalid command \N. Try \? for help.
regression=# \q
$ ls bozo
bozo not found
$

whereas formerly the backtick command would be evaluated.

Comments, objections?

regards, tom lane

Index: command.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.137
diff -c -r1.137 command.c
*** command.c	30 Nov 2004 20:00:34 -0000	1.137
--- command.c	18 Dec 2004 19:18:38 -0000
***************
*** 127,139 ****
  		status = CMD_ERROR;
  	}

! /* eat the rest of the options, if any */
! while ((arg = psql_scan_slash_option(scan_state,
! OT_NORMAL, NULL, false)))
{
! if (status != CMD_ERROR)
psql_error("\\%s: extra argument \"%s\" ignored\n", cmd, arg);
! free(arg);
}

  	/* if there is a trailing \\, swallow it */
--- 127,149 ----
  		status = CMD_ERROR;
  	}

! if (status != CMD_ERROR)
{
! /* eat any remaining arguments after a valid command */
! /* note we suppress evaluation of backticks here */
! while ((arg = psql_scan_slash_option(scan_state,
! OT_VERBATIM, NULL, false)))
! {
psql_error("\\%s: extra argument \"%s\" ignored\n", cmd, arg);
! free(arg);
! }
! }
! else
! {
! /* silently throw away rest of line after an erroneous command */
! while ((arg = psql_scan_slash_option(scan_state,
! OT_WHOLE_LINE, NULL, false)))
! free(arg);
}

  	/* if there is a trailing \\, swallow it */
Index: psqlscan.h
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/psqlscan.h,v
retrieving revision 1.3
diff -c -r1.3 psqlscan.h
*** psqlscan.h	29 Aug 2004 05:06:54 -0000	1.3
--- psqlscan.h	18 Dec 2004 19:18:38 -0000
***************
*** 32,38 ****
  	OT_SQLID,					/* treat as SQL identifier */
  	OT_SQLIDHACK,				/* SQL identifier, but don't downcase */
  	OT_FILEPIPE,				/* it's a filename or pipe */
! 	OT_WHOLE_LINE				/* just snarf the rest of the line */
  };

--- 32,39 ----
  	OT_SQLID,					/* treat as SQL identifier */
  	OT_SQLIDHACK,				/* SQL identifier, but don't downcase */
  	OT_FILEPIPE,				/* it's a filename or pipe */
! 	OT_WHOLE_LINE,				/* just snarf the rest of the line */
! 	OT_VERBATIM					/* literal (no backticks or variables) */
  };

Index: psqlscan.l
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/psqlscan.l,v
retrieving revision 1.7
diff -c -r1.7 psqlscan.l
*** psqlscan.l	29 Aug 2004 04:13:02 -0000	1.7
--- psqlscan.l	18 Dec 2004 19:18:38 -0000
***************
*** 723,746 ****
  				}

"`" {
! *option_quote = '`';
! BEGIN(xslashbackquote);
}

:[A-Za-z0-9_]* {
/* Possible psql variable substitution */
! const char *value;

! value = GetVariable(pset.vars, yytext + 1);

! /*
! * The variable value is just emitted without any
! * further examination. This is consistent with the
! * pre-8.0 code behavior, if not with the way that
! * variables are handled outside backslash commands.
! */
! if (value)
! appendPQExpBufferStr(output_buf, value);

*option_quote = ':';

--- 723,760 ----
  				}

"`" {
! if (option_type == OT_VERBATIM)
! {
! /* in verbatim mode, backquote is not special */
! ECHO;
! BEGIN(xslashdefaultarg);
! }
! else
! {
! *option_quote = '`';
! BEGIN(xslashbackquote);
! }
}

:[A-Za-z0-9_]* {
/* Possible psql variable substitution */
! if (option_type == OT_VERBATIM)
! ECHO;
! else
! {
! const char *value;

! value = GetVariable(pset.vars, yytext + 1);

*option_quote = ':';