From 098544473f33975da19b7463ce01fa27540c8cfc Mon Sep 17 00:00:00 2001
From: Ilia Ross <ilia@ross.gdn>
Date: Tue, 10 Mar 2026 17:29:17 +0200
Subject: [PATCH] Fix PostgreSQL user/group SQL quoting with shared helpers
 (#9)

---
 postgresql/postgresql-lib.pl | 26 +++++++++++++--
 postgresql/save_group.cgi    | 63 +++++++++++++++++++++++++++++-------
 postgresql/save_user.cgi     | 24 ++++++++------
 3 files changed, 89 insertions(+), 24 deletions(-)

diff --git a/postgresql/postgresql-lib.pl b/postgresql/postgresql-lib.pl
index ff1a08767..2e4a0f952 100755
--- a/postgresql/postgresql-lib.pl
+++ b/postgresql/postgresql-lib.pl
@@ -776,15 +776,37 @@ if (!-d $dir) {
 	}
 }
 
+# quote_table(table)
+# Quotes a table name, including any schema
 sub quote_table
 {
 local @tn = split(/\./, $_[0]);
-return join(".", map { "\"$_\"" } @tn);
+return join(".", map { &pg_quote_ident($_) } @tn);
 }
 
+# quotestr(string)
+# Quotes a string for use in SQL statements
 sub quotestr
 {
-return "\"$_[0]\"";
+return &pg_quote_ident($_[0]);
+}
+
+# pg_quote_ident(name)
+# Quotes an identifier for use in SQL statements
+sub pg_quote_ident
+{
+my ($name) = @_;
+$name =~ s/"/""/g;
+return "\"$name\"";
+}
+
+# pg_quote_lit(string)
+# Quotes a literal for use in SQL statements
+sub pg_quote_lit
+{
+my ($value) = @_;
+$value =~ s/'/''/g;
+return "'$value'";
 }
 
 # execute_sql_file(database, file, [user, pass], [unix-user])
diff --git a/postgresql/save_group.cgi b/postgresql/save_group.cgi
index 0b00e15f4..462d908ed 100755
--- a/postgresql/save_group.cgi
+++ b/postgresql/save_group.cgi
@@ -10,18 +10,28 @@ $access{'users'} || &error($text{'group_ecannot'});
 if ($in{'delete'}) {
 	# just delete the group
 	if (&get_postgresql_version() >= 8.0) {
-		&execute_sql_logged($config{'basedb'}, "drop group $in{'oldname'}");
+		$in{'oldname'} =~ /^[A-Za-z0-9_]+$/ || &error($text{'group_ename'});
+		&execute_sql_logged($config{'basedb'},
+				    "drop group ".&pg_quote_ident($in{'oldname'}));
 		}
 	else {
-		&execute_sql_logged($config{'basedb'}, "delete from pg_group where grosysid = $in{'gid'}");
+		$in{'gid'} =~ /^\d+$/ || &error($text{'group_egid'});
+		&execute_sql_logged($config{'basedb'},
+				    "delete from pg_group where grosysid = ?",
+				    $in{'gid'});
 		}
 	&webmin_log("delete", "group", $in{'name'});
 	}
 else {
 	# parse inputs
-	$in{'name'} =~ /^\S+$/ || &error($text{'group_ename'});
-	$s = &execute_sql($config{'basedb'}, "select * from pg_group where groname = '$in{'name'}'");
+	$in{'name'} =~ /^[A-Za-z0-9_]+$/ || &error($text{'group_ename'});
+	$s = &execute_sql($config{'basedb'},
+			  "select * from pg_group where groname = ?",
+			  $in{'name'});
 	$in{'gid'} =~ /^\d+$/ || &error($text{'group_egid'});
+	if (!$in{'new'}) {
+		$in{'oldname'} =~ /^[A-Za-z0-9_]+$/ || &error($text{'group_ename'});
+		}
 	if ($in{'new'}) {
 		$s->{'data'}->[0]->[0] && &error($text{'group_etaken'});
 		}
@@ -38,25 +48,49 @@ else {
 			$umap{$u->[1]} = $u->[0];
 			}
 		@mems = split(/\r?\n/, $in{'mems'});
+		foreach my $m (@mems) {
+			defined($umap{$m}) ||
+				&error("Invalid group member selected");
+			}
 		if ($in{'new'}) {
 			$first = shift(@mems);
-			&execute_sql_logged($config{'basedb'}, "create group $in{'name'} sysid $in{'gid'} user ".$umap{$first});
+			my $csql = "create group ".&pg_quote_ident($in{'name'}).
+				   " sysid $in{'gid'}";
+			$csql .= " user ".&pg_quote_ident($umap{$first})
+				if (defined($first));
+			&execute_sql_logged($config{'basedb'}, $csql);
 			if (@mems) {
-				&execute_sql_logged($config{'basedb'}, "alter group $in{'name'} add user ".join(" , ", map { $umap{$_} } @mems));
+				&execute_sql_logged($config{'basedb'},
+					"alter group ".&pg_quote_ident($in{'name'}).
+					" add user ".
+					join(" , ", map { &pg_quote_ident($umap{$_}) } @mems));
 				}
 			}
 		else {
 			if ($in{'name'} ne $in{'oldname'}) {
 				# Rename first
-				&execute_sql_logged($config{'basedb'}, "alter group $in{'oldname'} rename to $in{'name'}");
+				&execute_sql_logged($config{'basedb'},
+					"alter group ".&pg_quote_ident($in{'oldname'}).
+					" rename to ".&pg_quote_ident($in{'name'}));
 				}
-			$s = &execute_sql($config{'basedb'}, "select * from pg_group where groname = '$in{'name'}'");
+			$s = &execute_sql($config{'basedb'},
+					  "select * from pg_group where groname = ?",
+					  $in{'name'});
 			@oldmems = &split_array($s->{'data'}->[0]->[2]);
+			my @dropmems = grep { defined($_) && $_ ne '' }
+				       map { $umap{$_} } @oldmems;
 			if (@oldmems && $oldmems[0] ne '') {
-				&execute_sql_logged($config{'basedb'}, "alter group $in{'name'} drop user ".join(", ", map { $umap{$_} } @oldmems));
+				&execute_sql_logged($config{'basedb'},
+					"alter group ".&pg_quote_ident($in{'name'}).
+					" drop user ".
+					join(", ", map { &pg_quote_ident($_) } @dropmems))
+					if (@dropmems);
 				}
 			if (@mems) {
-				&execute_sql_logged($config{'basedb'}, "alter group $in{'name'} add user ".join(" , ", map { $umap{$_} } @mems));
+				&execute_sql_logged($config{'basedb'},
+					"alter group ".&pg_quote_ident($in{'name'}).
+					" add user ".
+					join(" , ", map { &pg_quote_ident($umap{$_}) } @mems));
 				}
 			}
 		}
@@ -64,10 +98,15 @@ else {
 		# Can update group table directly
 		$mems = &join_array(split(/\0/, $in{'mems'}));
 		if ($in{'new'}) {
-			&execute_sql_logged($config{'basedb'}, "insert into pg_group values ('$in{'name'}', '$in{'gid'}', '$mems')");
+			&execute_sql_logged($config{'basedb'},
+					    "insert into pg_group values (?, ?, ?)",
+					    $in{'name'}, $in{'gid'}, $mems);
 			}
 		else {
-			&execute_sql_logged($config{'basedb'}, "update pg_group set groname = '$in{'name'}', grolist = '$mems' where grosysid = $in{'gid'}");
+			&execute_sql_logged($config{'basedb'},
+					    "update pg_group set groname = ?, ".
+					    "grolist = ? where grosysid = ?",
+					    $in{'name'}, $mems, $in{'gid'});
 			}
 		}
 	&webmin_log($in{'new'} ? "create" : "modify", "group", $in{'name'});
diff --git a/postgresql/save_user.cgi b/postgresql/save_user.cgi
index 0d8cbdcd6..1ac33ccac 100755
--- a/postgresql/save_user.cgi
+++ b/postgresql/save_user.cgi
@@ -9,19 +9,24 @@ $access{'users'} || &error($text{'user_ecannot'});
 
 if ($in{'delete'}) {
 	# just delete the user
-	$main::disable_postgresql_escaping = 1;
-	&execute_sql_logged($config{'basedb'}, "drop user \"$in{'user'}\"");
+	$in{'user'} =~ /^[A-Za-z0-9_]+$/ || &error($text{'user_ename'});
+	&execute_sql_logged($config{'basedb'},
+			    "drop user ".&pg_quote_ident($in{'user'}));
 	&webmin_log("delete", "user", $in{'user'});
 	}
 else {
 	# parse inputs
+	$in{'pname'} =~ /^[A-Za-z0-9_]+$/ || &error($text{'user_ename'});
+	if (!$in{'new'}) {
+		$in{'user'} =~ /^[A-Za-z0-9_]+$/ || &error($text{'user_ename'});
+		}
 	$version = &get_postgresql_version();
 	if ($in{'ppass_def'} == 0) {
 		$in{'ppass'} =~ /^\S+$/ || &error($text{'user_epass'});
-		$sql .= $version >= 7 ? " with password '$in{'ppass'}'"
+		$sql .= $version >= 7 ? " with password ".&pg_quote_lit($in{'ppass'})
 				      : " with password $in{'ppass'}";
 		}
-	elsif ($in{'pass_def'} == 1) {
+	elsif ($in{'ppass_def'} == 1) {
 		$sql .= " with password ''";
 		}
 	if ($in{'db'}) {
@@ -39,23 +44,22 @@ else {
 			}
 		}
 	if (!$in{'until_def'}) {
-		$sql .= " valid until '$in{'until'}'";
+		$sql .= " valid until ".&pg_quote_lit($in{'until'});
 		}
 	if ($in{'new'}) {
-		$in{'pname'} =~ /^\S+$/ || &error($text{'user_ename'});
 		&execute_sql_logged($config{'basedb'},
-				    "create user \"$in{'pname'}\" $sql");
+				    "create user ".&pg_quote_ident($in{'pname'})." $sql");
 		&webmin_log("create", "user", $in{'pname'});
 		}
 	else {
 		&execute_sql_logged($config{'basedb'},
-				    "alter user \"$in{'user'}\" $sql");
+				    "alter user ".&pg_quote_ident($in{'user'})." $sql");
 		if (&get_postgresql_version() >= 7.4 &&
 		    $in{'pname'} ne $in{'user'}) {
 			# Rename too
 			&execute_sql_logged($config{'basedb'},
-		    		"alter user \"$in{'user'}\" ".
-				"rename to \"$in{'pname'}\"");
+		    		"alter user ".&pg_quote_ident($in{'user'})." ".
+				"rename to ".&pg_quote_ident($in{'pname'}));
 			}
 		&webmin_log("modify", "user", $in{'user'});
 		}