Replace a bunch more uses of strncpy() with safer coding

Enterprise / PostgreSQL - Tom Lane [sss.pgh.pa.us] - 24 January 2015 12:05 UTC

strncpy() has a well-deserved reputation for being unsafe, so make an effort to get rid of nearly all occurrences in HEAD.

A large fraction of the remaining uses were passing length less than or equal to the known strlen() of the source, in which case no null-padding can occur and the behavior is equivalent to memcpy(), though doubtless slower and certainly harder to reason about. So just use memcpy() in these cases.

In other cases, use either StrNCpy() or strlcpy() as appropriate (depending on whether padding to the full length of the destination buffer seems useful).

I left a few strncpy() calls alone in the src/timezone/ code, to keep it in sync with upstream (the IANA tzcode distribution). There are also a few such calls in ecpg that could possibly do with more analysis.

AFAICT, none of these changes are more than cosmetic, except for the four occurrences in fe-secure-openssl.c, which are in fact buggy: an overlength source leads to a non-null-terminated destination buffer and ensuing misbehavior. These don't seem like security issues, first because no stack clobber is possible and second because if your values of sslcert etc are coming from untrusted sources then you've got problems way worse than this. Still, it's undesirable to have unpredictable behavior for overlength inputs, so back-patch those four changes to all active branches.

586dd5d Replace a bunch more uses of strncpy() with safer coding.
contrib/fuzzystrmatch/dmetaphone.c | 2 +-
contrib/isn/isn.c | 8 ++++----
contrib/pg_trgm/trgm_regexp.c | 2 +-
contrib/pgbench/pgbench.c | 2 +-
contrib/pgcrypto/crypt-des.c | 3 +--
contrib/xml2/xpath.c | 2 +-
src/backend/libpq/hba.c | 7 +++++--
src/backend/postmaster/pgstat.c | 2 +-
src/backend/regex/regerror.c | 2 +-
src/backend/replication/logical/logical.c | 4 +---
src/backend/replication/slot.c | 3 +--
src/backend/utils/adt/datetime.c | 2 +-
src/backend/utils/adt/name.c | 2 +-
src/backend/utils/adt/varlena.c | 2 +-
src/backend/utils/error/elog.c | 2 +-
src/bin/initdb/initdb.c | 4 ++--
src/bin/pg_dump/pg_backup_archiver.c | 2 +-
src/bin/pg_dump/pg_backup_db.c | 2 +-
src/interfaces/ecpg/ecpglib/execute.c | 14 +++++++-------
src/interfaces/ecpg/ecpglib/prepare.c | 2 +-
src/interfaces/ecpg/pgtypeslib/datetime.c | 10 +++++-----
src/interfaces/ecpg/pgtypeslib/dt_common.c | 2 +-
src/interfaces/ecpg/test/pg_regress_ecpg.c | 3 +--
src/interfaces/libpq/fe-exec.c | 6 +++---
src/interfaces/libpq/fe-protocol2.c | 1 +
src/interfaces/libpq/fe-secure-openssl.c | 8 ++++----
26 files changed, 49 insertions(+), 50 deletions(-)

Upstream: git.postgresql.org

  • Share