public inbox for [email protected]  
help / color / mirror / Atom feed
From: SATYANARAYANA NARLAPURAM <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Cc: jian he <[email protected]>
Subject: COPY FROM ON_ERROR SET_NULL bypasses domain NOT NULL with partial column list
Date: Thu, 16 Apr 2026 10:09:40 -0700
Message-ID: <CAHg+QDdej0c0gWJi2FnbirzhgzyZNPiTwC1P5B_-dSNCzq-91A@mail.gmail.com> (raw)

HI hackers,

domain_with_constraint[] was allocated with list_length(attnumlist)
elements and indexed sequentially via foreach_current_index(), but
copyfromparse.c accesses it via attnum - 1 (physical attribute index).
With a partial column list targeting high-numbered columns, this caused
an out-of-bounds read that bypassed domain NOT NULL checks, silently
inserting NULL into NOT NULL domain columns.

Fix by allocating with num_phys_attrs and indexing by attnum - 1,
consistent with all other per-column arrays in BeginCopyFrom().

Patch is attached, and added a new test case to cover this scenario.

Repro:

CREATE DOMAIN d_notnull_int AS int NOT NULL;
CREATE TABLE t (
    c1 text, c2 text, c3 text, c4 text, c5 text,
    c6 text, c7 text, c8 text, c9 text,
    c10 d_notnull_int
);

COPY t(c1, c10) FROM stdin WITH (on_error set_null);
hello    bad
\.

SELECT c10 IS NULL FROM t;

Thanks,
Satya


Attachments:

  [application/octet-stream] 0001-Fix-COPY-FROM-ON_ERROR-SET_NULL-OOB-read-with-partia.patch (5.5K, 3-0001-Fix-COPY-FROM-ON_ERROR-SET_NULL-OOB-read-with-partia.patch)
  download | inline diff:
From ff9e5823164714e14cba5e70099c051bc2571e70 Mon Sep 17 00:00:00 2001
From: Satya Narlapuram <[email protected]>
Date: Thu, 16 Apr 2026 16:43:34 +0000
Subject: [PATCH] Fix COPY FROM ON_ERROR SET_NULL OOB read with partial column
 list

domain_with_constraint[] was allocated with list_length(attnumlist)
elements and indexed sequentially via foreach_current_index(), but
copyfromparse.c accesses it via attnum - 1 (physical attribute index).
With a partial column list targeting high-numbered columns, this caused
an out-of-bounds read that bypassed domain NOT NULL checks, silently
inserting NULL into NOT NULL domain columns.

Fix by allocating with num_phys_attrs and indexing by attnum - 1,
consistent with all other per-column arrays in BeginCopyFrom().
---
 src/backend/commands/copyfrom.c     |  8 ++------
 src/test/regress/expected/copy2.out | 23 +++++++++++++++++++++++
 src/test/regress/sql/copy2.sql      | 19 +++++++++++++++++++
 3 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 64ac3063..0087585b 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -1636,8 +1636,6 @@ BeginCopyFrom(ParseState *pstate,
 
 	if (cstate->opts.on_error == COPY_ON_ERROR_SET_NULL)
 	{
-		int			attr_count = list_length(cstate->attnumlist);
-
 		/*
 		 * When data type conversion fails and ON_ERROR is SET_NULL, we need
 		 * ensure that the input column allow null values.  ExecConstraints()
@@ -1646,15 +1644,13 @@ BeginCopyFrom(ParseState *pstate,
 		 * check must be performed during the initial string-to-datum
 		 * conversion (see CopyFromTextLikeOneRow()).
 		 */
-		cstate->domain_with_constraint = palloc0_array(bool, attr_count);
+		cstate->domain_with_constraint = palloc0_array(bool, num_phys_attrs);
 
 		foreach_int(attno, cstate->attnumlist)
 		{
-			int			i = foreach_current_index(attno);
-
 			Form_pg_attribute att = TupleDescAttr(tupDesc, attno - 1);
 
-			cstate->domain_with_constraint[i] = DomainHasConstraints(att->atttypid, NULL);
+			cstate->domain_with_constraint[attno - 1] = DomainHasConstraints(att->atttypid, NULL);
 		}
 	}
 
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index 7600e523..2d6bfdd8 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -839,6 +839,28 @@ SELECT * FROM t_on_error_null ORDER BY a;
  13 |   14 | NULL
 (3 rows)
 
+-- Test on_error set_null with partial column list and domain NOT NULL.
+-- Bug: domain_with_constraint[] was indexed by physical attnum, but
+-- allocated with only as many elements as the COPY column list.
+-- With a partial column list targeting a high-numbered column, this
+-- caused an out-of-bounds read that bypassed the domain NOT NULL check.
+CREATE TABLE t_on_error_null_partial (
+    c1 text, c2 text, c3 text, c4 text, c5 text,
+    c6 text, c7 text, c8 text, c9 text,
+    c10 d_int_not_null   -- attnum 10, domain NOT NULL
+);
+-- Partial column list: attnumlist = {1, 10}.  Should fail because
+-- the domain on c10 does not allow null, and 'bad' is not an integer.
+COPY t_on_error_null_partial(c1, c10) FROM STDIN WITH (on_error set_null); -- fail
+ERROR:  domain d_int_not_null does not allow null values
+DETAIL:  ON_ERROR SET_NULL cannot be applied because column "c10" (domain d_int_not_null) does not accept null values.
+CONTEXT:  COPY t_on_error_null_partial, line 1, column c10: "bad"
+SELECT count(*) AS "expect_0" FROM t_on_error_null_partial;
+ expect_0 
+----------
+        0
+(1 row)
+
 \pset null ''
 -- tests for on_error option with log_verbosity and null constraint via domain
 CREATE DOMAIN dcheck_ign_err2 varchar(15) NOT NULL;
@@ -900,6 +922,7 @@ DROP VIEW instead_of_insert_tbl_view_2;
 DROP FUNCTION fun_instead_of_insert_tbl();
 DROP TABLE check_ign_err;
 DROP TABLE t_on_error_null;
+DROP TABLE t_on_error_null_partial;
 DROP DOMAIN d_int_not_null;
 DROP DOMAIN d_int_positive_maybe_null;
 DROP TABLE check_ign_err2;
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index e0810109..287c8fb8 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -580,6 +580,24 @@ COPY t_on_error_null FROM STDIN WITH (on_error set_null, log_verbosity verbose);
 
 SELECT * FROM t_on_error_null ORDER BY a;
 
+-- Test on_error set_null with partial column list and domain NOT NULL.
+-- Bug: domain_with_constraint[] was indexed by physical attnum, but
+-- allocated with only as many elements as the COPY column list.
+-- With a partial column list targeting a high-numbered column, this
+-- caused an out-of-bounds read that bypassed the domain NOT NULL check.
+CREATE TABLE t_on_error_null_partial (
+    c1 text, c2 text, c3 text, c4 text, c5 text,
+    c6 text, c7 text, c8 text, c9 text,
+    c10 d_int_not_null   -- attnum 10, domain NOT NULL
+);
+-- Partial column list: attnumlist = {1, 10}.  Should fail because
+-- the domain on c10 does not allow null, and 'bad' is not an integer.
+COPY t_on_error_null_partial(c1, c10) FROM STDIN WITH (on_error set_null); -- fail
+hello	bad
+\.
+
+SELECT count(*) AS "expect_0" FROM t_on_error_null_partial;
+
 \pset null ''
 
 -- tests for on_error option with log_verbosity and null constraint via domain
@@ -652,6 +670,7 @@ DROP VIEW instead_of_insert_tbl_view_2;
 DROP FUNCTION fun_instead_of_insert_tbl();
 DROP TABLE check_ign_err;
 DROP TABLE t_on_error_null;
+DROP TABLE t_on_error_null_partial;
 DROP DOMAIN d_int_not_null;
 DROP DOMAIN d_int_positive_maybe_null;
 DROP TABLE check_ign_err2;
-- 
2.43.0



reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: COPY FROM ON_ERROR SET_NULL bypasses domain NOT NULL with partial column list
  In-Reply-To: <CAHg+QDdej0c0gWJi2FnbirzhgzyZNPiTwC1P5B_-dSNCzq-91A@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox