Fix misparsing of empty value in conninfo_uri_parse_params().

tglsfdc · tglsfdc · commit 83c3115dd0dd · 2015-02-21T12:59:43.000-05:00
After finding an "=" character, the pointer was advanced twice when it
should only advance once.  This is harmless as long as the value after "="
has at least one character; but if it doesn't, we'd miss the terminator
character and include too much in the value.

In principle this could lead to reading off the end of memory.  It does not
seem worth treating as a security issue though, because it would happen on
client side, and besides client logic that's taking conninfo strings from
untrusted sources has much worse security problems than this.

Report and patch received off-list from Thomas Fanghaenel.
Back-patch to 9.2 where the faulty code was introduced.
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
@@ -4959,9 +4959,8 @@ conninfo_uri_parse_params(char *params,
 					++p;
 				break;
 			}
-
-			/* Advance, NUL is checked in the 'if' above */
-			++p;
+			else
+				++p;			/* Advance over all other bytes. */
 		}
 
 		keyword = conninfo_uri_decode(keyword, errorMessage);

Original file line number	Diff line number	Diff line change
`@@ -4959,9 +4959,8 @@ conninfo_uri_parse_params(char *params,`
`4959`	`4959`	`++p;`
`4960`	`4960`	`break;`
`4961`	`4961`	`}`
`4962`		`-`
`4963`		`- /* Advance, NUL is checked in the 'if' above */`
`4964`		`- ++p;`
	`4962`	`+ else`
	`4963`	`+ ++p; /* Advance over all other bytes. */`
`4965`	`4964`	`}`
`4966`	`4965`
`4967`	`4966`	`keyword = conninfo_uri_decode(keyword, errorMessage);`