To: pgsql-hackers <pgsql-hackers@postgresql.org>
From: Alexander Lakhin <exclusion@gmail.com>
Subject: Prevent invalid memory access in LookupFuncName
Message-ID: <3d068be5-f617-a5ee-99f6-458a407bfd65@gmail.com>
Date: Mon, 24 Jun 2019 23:10:54 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.0
MIME-Version: 1.0
Content-Type: multipart/mixed;
 boundary="------------6CBF33094F52BFA510C5A9A3"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------6CBF33094F52BFA510C5A9A3
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Hello hackers,

When running on REL_11_STABLE the following query:
CREATE PROCEDURE test_ambiguous_procname(int) as $$ begin end; $$
language plpgsql;
CREATE PROCEDURE test_ambiguous_procname(text) as $$ begin end; $$
language plpgsql;
DROP PROCEDURE test_ambiguous_procname;
under valgrind I get the memory access errors:

2019-06-24 22:21:39.925 MSK|law|regression|5d1122c2.2921|LOG: 
statement: DROP PROCEDURE test_ambiguous_procname;
==00:00:00:07.756 10529== Conditional jump or move depends on
uninitialised value(s)
==00:00:00:07.756 10529==    at 0x4C35E60: __memcmp_sse4_1 (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==00:00:00:07.756 10529==    by 0x2E9A7B: LookupFuncName (parse_func.c:2078)
==00:00:00:07.756 10529==    by 0x2E9D96: LookupFuncWithArgs
(parse_func.c:2141)
==00:00:00:07.756 10529==    by 0x2A2F7E: get_object_address
(objectaddress.c:893)
==00:00:00:07.756 10529==    by 0x31C9C0: RemoveObjects (dropcmds.c:71)
==00:00:00:07.756 10529==    by 0x50D4FF: ExecDropStmt (utility.c:1738)
==00:00:00:07.756 10529==    by 0x5100BC: ProcessUtilitySlow
(utility.c:1580)
==00:00:00:07.756 10529==    by 0x50EDFE: standard_ProcessUtility
(utility.c:835)
==00:00:00:07.756 10529==    by 0x50F07D: ProcessUtility (utility.c:360)
==00:00:00:07.756 10529==    by 0x50B4D2: PortalRunUtility (pquery.c:1178)
==00:00:00:07.756 10529==    by 0x50C169: PortalRunMulti (pquery.c:1324)
==00:00:00:07.756 10529==    by 0x50CEFF: PortalRun (pquery.c:799)
==00:00:00:07.756 10529==  Uninitialised value was created by a stack
allocation
==00:00:00:07.756 10529==    at 0x2E9C31: LookupFuncWithArgs
(parse_func.c:2106)
==00:00:00:07.756 10529==
...
==00:00:00:07.756 10529== Conditional jump or move depends on
uninitialised value(s)
==00:00:00:07.756 10529==    at 0x2E9A7E: LookupFuncName (parse_func.c:2078)
==00:00:00:07.756 10529==    by 0x2E9D96: LookupFuncWithArgs
(parse_func.c:2141)
==00:00:00:07.757 10529==    by 0x2A2F7E: get_object_address
(objectaddress.c:893)
==00:00:00:07.757 10529==    by 0x31C9C0: RemoveObjects (dropcmds.c:71)
==00:00:00:07.757 10529==    by 0x50D4FF: ExecDropStmt (utility.c:1738)
==00:00:00:07.757 10529==    by 0x5100BC: ProcessUtilitySlow
(utility.c:1580)
==00:00:00:07.757 10529==    by 0x50EDFE: standard_ProcessUtility
(utility.c:835)
==00:00:00:07.757 10529==    by 0x50F07D: ProcessUtility (utility.c:360)
==00:00:00:07.757 10529==    by 0x50B4D2: PortalRunUtility (pquery.c:1178)
==00:00:00:07.757 10529==    by 0x50C169: PortalRunMulti (pquery.c:1324)
==00:00:00:07.757 10529==    by 0x50CEFF: PortalRun (pquery.c:799)
==00:00:00:07.757 10529==    by 0x5090FF: exec_simple_query
(postgres.c:1145)
==00:00:00:07.757 10529==  Uninitialised value was created by a stack
allocation
==00:00:00:07.757 10529==    at 0x2E9C31: LookupFuncWithArgs
(parse_func.c:2106)

As I see, the code in LookupFuncName can fall through the "if (nargs ==
-1)" condition and execute memcmp with nargs==-1.
The proposed patch is attached.

Best regards,
Alexander

--------------6CBF33094F52BFA510C5A9A3
Content-Type: text/x-patch;
 name="fix-LookupFuncName.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="fix-LookupFuncName.patch"

diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 44257154b81..ee9f73ef977 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2071,6 +2071,7 @@ LookupFuncName(List *funcname, int nargs, const Oid *argtypes, bool noError)
 						 errmsg("could not find a function named \"%s\"",
 								NameListToString(funcname))));
 		}
+		return InvalidOid;
 	}
 
 	while (clist)

--------------6CBF33094F52BFA510C5A9A3--