Neil Conway <[EMAIL PROTECTED]> writes:
> I'll put this on the back-burner for now, and repost a complete
> patch later if I get around to it.
I've applied the following patch (since I'd already gone ahead and
done the work) that replaces appendStringInfo(buf, "%s", str) with
appendStringInfoString(buf, str)
It occurred to me that there is a potential security problem with code
like:
char *my_str;
my_str = read_from_an_untrusted_source();
appendStringInfo(buf, my_str);
If my_str contains any formatting characters, this crashes the
backend. I'm not sure if there are any actual exploitable instances of
this in the backend, but the above unsafe coding practise is fairly
common.
-Neil
Index: src/backend/commands/explain.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/backend/commands/explain.c,v
retrieving revision 1.118
diff -c -r1.118 explain.c
*** src/backend/commands/explain.c 29 Nov 2003 19:51:47 -0000 1.118
--- src/backend/commands/explain.c 31 Jan 2004 04:49:13 -0000
***************
*** 1002,1008 ****
/* And add to str */
if (keyno > 0)
appendStringInfo(str, ", ");
! appendStringInfo(str, "%s", exprstr);
}
appendStringInfo(str, "\n");
--- 1002,1008 ----
/* And add to str */
if (keyno > 0)
appendStringInfo(str, ", ");
! appendStringInfoString(str, exprstr);
}
appendStringInfo(str, "\n");
Index: src/backend/nodes/outfuncs.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/backend/nodes/outfuncs.c,v
retrieving revision 1.231
diff -c -r1.231 outfuncs.c
*** src/backend/nodes/outfuncs.c 22 Jan 2004 00:34:31 -0000 1.231
--- src/backend/nodes/outfuncs.c 31 Jan 2004 04:49:38 -0000
***************
*** 1427,1433 ****
* We assume the value is a valid numeric literal and so does
* not need quoting.
*/
! appendStringInfo(str, "%s", value->val.str);
break;
case T_String:
appendStringInfoChar(str, '"');
--- 1427,1433 ----
* We assume the value is a valid numeric literal and so does
* not need quoting.
*/
! appendStringInfoString(str, value->val.str);
break;
case T_String:
appendStringInfoChar(str, '"');
***************
*** 1436,1442 ****
break;
case T_BitString:
/* internal representation already has leading 'b' */
! appendStringInfo(str, "%s", value->val.str);
break;
default:
elog(ERROR, "unrecognized node type: %d", (int) value->type);
--- 1436,1442 ----
break;
case T_BitString:
/* internal representation already has leading 'b' */
! appendStringInfoString(str, value->val.str);
break;
default:
elog(ERROR, "unrecognized node type: %d", (int) value->type);
Index: src/backend/utils/adt/regproc.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/backend/utils/adt/regproc.c,v
retrieving revision 1.85
diff -c -r1.85 regproc.c
*** src/backend/utils/adt/regproc.c 29 Nov 2003 19:51:59 -0000 1.85
--- src/backend/utils/adt/regproc.c 31 Jan 2004 04:49:56 -0000
***************
*** 340,346 ****
if (i > 0)
appendStringInfoChar(&buf, ',');
! appendStringInfo(&buf, "%s", format_type_be(thisargtype));
}
appendStringInfoChar(&buf, ')');
--- 340,346 ----
if (i > 0)
appendStringInfoChar(&buf, ',');
! appendStringInfoString(&buf, format_type_be(thisargtype));
}
appendStringInfoChar(&buf, ')');
Index: src/backend/utils/adt/ruleutils.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/backend/utils/adt/ruleutils.c,v
retrieving revision 1.161
diff -c -r1.161 ruleutils.c
*** src/backend/utils/adt/ruleutils.c 28 Dec 2003 21:57:37 -0000 1.161
--- src/backend/utils/adt/ruleutils.c 31 Jan 2004 04:55:19 -0000
***************
*** 752,758 ****
attname = get_relid_attribute_name(indrelid, attnum);
if (!colno || colno == keyno + 1)
! appendStringInfo(&buf, "%s", quote_identifier(attname));
keycoltype = get_atttype(indrelid, attnum);
}
else
--- 752,758 ----
attname = get_relid_attribute_name(indrelid, attnum);
if (!colno || colno == keyno + 1)
! appendStringInfoString(&buf, quote_identifier(attname));
keycoltype = get_atttype(indrelid, attnum);
}
else
***************
*** 772,778 ****
/* Need parens if it's not a bare function call */
if (indexkey && IsA(indexkey, FuncExpr) &&
((FuncExpr *) indexkey)->funcformat == COERCE_EXPLICIT_CALL)
! appendStringInfo(&buf, "%s", str);
else
appendStringInfo(&buf, "(%s)", str);
}
--- 772,778 ----
/* Need parens if it's not a bare function call */
if (indexkey && IsA(indexkey, FuncExpr) &&
((FuncExpr *) indexkey)->funcformat == COERCE_EXPLICIT_CALL)
! appendStringInfoString(&buf, str);
else
appendStringInfo(&buf, "(%s)", str);
}
***************
*** 947,953 ****
string = ""; /* keep compiler quiet */
break;
}
! appendStringInfo(&buf, "%s", string);
/* Add ON UPDATE and ON DELETE clauses, if needed */
switch (conForm->confupdtype)
--- 947,953 ----
string = ""; /* keep compiler quiet */
break;
}
! appendStringInfoString(&buf, string);
/* Add ON UPDATE and ON DELETE clauses, if needed */
switch (conForm->confupdtype)
***************
*** 1126,1136 ****
colName = get_relid_attribute_name(relId, DatumGetInt16(keys[j]));
if (j == 0)
! appendStringInfo(buf, "%s",
! quote_identifier(colName));
else
! appendStringInfo(buf, ", %s",
! quote_identifier(colName));
}
}
--- 1126,1134 ----
colName = get_relid_attribute_name(relId, DatumGetInt16(keys[j]));
if (j == 0)
! appendStringInfoString(buf, quote_identifier(colName));
else
! appendStringInfo(buf, ", %s", quote_identifier(colName));
}
}
***************
*** 2134,2142 ****
appendStringInfo(buf, sep);
sep = ", ";
! appendStringInfo(buf, "%s",
! quote_identifier(get_relid_attribute_name(rte->relid,
! tle->resdom->resno)));
}
appendStringInfo(buf, ") ");
--- 2132,2140 ----
appendStringInfo(buf, sep);
sep = ", ";
! appendStringInfoString(buf,
! quote_identifier(get_relid_attribute_name(rte->relid,
! tle->resdom->resno)));
}
appendStringInfo(buf, ") ");
***************
*** 2753,2759 ****
quote_identifier(refname));
}
if (attname)
! appendStringInfo(buf, "%s", quote_identifier(attname));
else
appendStringInfo(buf, "*");
}
--- 2751,2757 ----
quote_identifier(refname));
}
if (attname)
! appendStringInfoString(buf, quote_identifier(attname));
else
appendStringInfo(buf, "*");
}
***************
*** 3763,3770 ****
{
if (col != rte->alias->colnames)
appendStringInfo(buf, ", ");
! appendStringInfo(buf, "%s",
! quote_identifier(strVal(lfirst(col))));
}
appendStringInfoChar(buf, ')');
}
--- 3761,3768 ----
{
if (col != rte->alias->colnames)
appendStringInfo(buf, ", ");
! appendStringInfoString(buf,
! quote_identifier(strVal(lfirst(col))));
}
appendStringInfoChar(buf, ')');
}
***************
*** 3902,3909 ****
{
if (col != j->using)
appendStringInfo(buf, ", ");
! appendStringInfo(buf, "%s",
! quote_identifier(strVal(lfirst(col))));
}
appendStringInfoChar(buf, ')');
}
--- 3900,3907 ----
{
if (col != j->using)
appendStringInfo(buf, ", ");
! appendStringInfoString(buf,
! quote_identifier(strVal(lfirst(col))));
}
appendStringInfoChar(buf, ')');
}
***************
*** 3934,3940 ****
{
if (col != j->alias->colnames)
appendStringInfo(buf, ", ");
! appendStringInfo(buf, "%s",
quote_identifier(strVal(lfirst(col))));
}
appendStringInfoChar(buf, ')');
--- 3932,3938 ----
{
if (col != j->alias->colnames)
appendStringInfo(buf, ", ");
! appendStringInfoString(buf,
quote_identifier(strVal(lfirst(col))));
}
appendStringInfoChar(buf, ')');
***************
*** 4164,4170 ****
initStringInfo(&buf);
if (namespace)
appendStringInfo(&buf, "%s.", quote_identifier(namespace));
! appendStringInfo(&buf, "%s", quote_identifier(ident));
return buf.data;
}
--- 4162,4168 ----
initStringInfo(&buf);
if (namespace)
appendStringInfo(&buf, "%s.", quote_identifier(namespace));
! appendStringInfoString(&buf, quote_identifier(ident));
return buf.data;
}
***************
*** 4316,4322 ****
appendStringInfo(&buf, "OPERATOR(%s.", quote_identifier(nspname));
}
! appendStringInfo(&buf, "%s", oprname);
if (nspname)
appendStringInfoChar(&buf, ')');
--- 4314,4320 ----
appendStringInfo(&buf, "OPERATOR(%s.", quote_identifier(nspname));
}
! appendStringInfoString(&buf, oprname);
if (nspname)
appendStringInfoChar(&buf, ')');
***************
*** 4338,4344 ****
int nnames = length(opname);
if (nnames == 1)
! appendStringInfo(buf, "%s", strVal(lfirst(opname)));
else
{
appendStringInfo(buf, "OPERATOR(");
--- 4336,4342 ----
int nnames = length(opname);
if (nnames == 1)
! appendStringInfoString(buf, strVal(lfirst(opname)));
else
{
appendStringInfo(buf, "OPERATOR(");
Index: src/backend/utils/adt/varlena.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/backend/utils/adt/varlena.c,v
retrieving revision 1.109
diff -c -r1.109 varlena.c
*** src/backend/utils/adt/varlena.c 19 Dec 2003 04:56:41 -0000 1.109
--- src/backend/utils/adt/varlena.c 31 Jan 2004 04:56:21 -0000
***************
*** 2213,2219 ****
if (i > 0)
appendStringInfo(result_str, "%s%s", fldsep, value);
else
! appendStringInfo(result_str, "%s", value);
p = att_addlength(p, typlen, PointerGetDatum(p));
p = (char *) att_align(p, typalign);
--- 2213,2219 ----
if (i > 0)
appendStringInfo(result_str, "%s%s", fldsep, value);
else
! appendStringInfoString(result_str, value);
p = att_addlength(p, typlen, PointerGetDatum(p));
p = (char *) att_align(p, typalign);
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /var/lib/cvs/pgsql-server/src/backend/utils/misc/guc.c,v
retrieving revision 1.181
diff -c -r1.181 guc.c
*** src/backend/utils/misc/guc.c 26 Jan 2004 22:35:32 -0000 1.181
--- src/backend/utils/misc/guc.c 31 Jan 2004 04:57:04 -0000
***************
*** 3259,3265 ****
break;
case T_Float:
/* represented as a string, so just copy it */
! appendStringInfo(&buf, "%s", strVal(&arg->val));
break;
case T_String:
val = strVal(&arg->val);
--- 3259,3265 ----
break;
case T_Float:
/* represented as a string, so just copy it */
! appendStringInfoString(&buf, strVal(&arg->val));
break;
case T_String:
val = strVal(&arg->val);
***************
*** 3293,3301 ****
* mode, quote it if it's not a vanilla identifier.
*/
if (flags & GUC_LIST_QUOTE)
! appendStringInfo(&buf, "%s", quote_identifier(val));
else
! appendStringInfo(&buf, "%s", val);
}
break;
default:
--- 3293,3301 ----
* mode, quote it if it's not a vanilla identifier.
*/
if (flags & GUC_LIST_QUOTE)
! appendStringInfoString(&buf, quote_identifier(val));
else
! appendStringInfoString(&buf, val);
}
break;
default:
---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
