Reject Windows command lines not convertible to system locale

From: Noah Misch <noah(at)leadboat(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Reject Windows command lines not convertible to system locale
Date: 2024-12-15 02:27:01
Message-ID: 20241215022701.a1.nmisch@google.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The security team received a report about misbehavior when a Windows program
executes a PostgreSQL program, passing a wide character on the command line.
Every such character gets translated to something else. Of note, a U+FF02
FULLWIDTH QUOTATION MARK in the caller-provided command line becomes U+0022
QUOTATION MARK in GetCommandLineA(). Since U+0022 is a metacharacter for
splitting the command line into argv, the argv argument boundaries may be
contrary to caller intentions. This isn't a vulnerability in PostgreSQL, but
it may be a vulnerability in Windows programs that construct CreateProcessW()
arguments to execute PostgreSQL programs. We don't know of any affected
programs, but we didn't search diligently.

The report (reporter bcc'd) proposed a fix of using non-ANSI APIs (UNICODE
APIs) to process the command line. That would create the worse problem that
both the caller (runs CreateProcessW() or CreateProcessA()) and callee (runs
GetCommandLineA() or GetCommandLineW()) would need to change simultaneously or
suffer mojibake. Consider the example of the argument to vacuumdb's
"--schema" argument. Under PGCLIENTENCODING=UTF8, a caller wanting YEN SIGN
runs CreateProcessA(... 0xC2 0xA5 ...) or
CreateProcessW(... MultiByteToWideChar(CP_ACP, 0, ... 0xC2 0xA5 ...), ...).
Suppose vacuumdb switched to wmain(int argc, wchar_t *argv[]) and interpreted
argv as UTF-16. The same two CreateProcess calls would then get a
two-character schema name instead, yielding mojibake until callers update to
CreateProcessW(... 0x00A5 ...). That would create as many vulnerabilities as
it solves.

What we can do safely is exit if we could not convert the command line to the
Windows ANSI code page (losslessly). Patch attached.

As I mention in a code comment, I decided not to do anything about the similar
hazard in GetEnvironmentStringsW(). A possible alternative would be to check
each var referenced in PostgreSQL code or each var where the name begins with
"PG". Should we use one of those or another alternative?

The Perl testing calls powershell, which is new for our source tree. The
Internet thinks Windows Server 2008 R1 is the newest Windows where powershell
was optional. The buildfarm's oldest Windows is Windows Server 2008 R2. If
powershell is bad, there's likely a hairy way to write the test in any of
cmd.exe, cscript, Perl Win32::API, or C. My first choice was Perl
Win32::Process or IPC::Run. This test needs CreateProcessW(), but those two
use CreateProcessA().

I subjected the postmaster to program_options_handling_ok() and the other
tests we apply to most executables.

I didn't test a CP_UTF8 Windows system. The new test coverage has a condition
intended to avoid failure there. (My Windows development environments are all
too old, and I stopped short of building a new one for this.) CP_UTF8 Windows
raises broader issues. I'll start a separate thread about them.

I wanted to put the check in some function that most main() instances already
call, instead of adding ~40 new calls for this esoteric topic. Options:

1. Call in pg_logging_init() and the backend. Add pg_logging_init() calls to
pg_config, ecpg (the ecpg preprocessor), and pg_test_timing. I picked
this, for two minor benefits. First, it facilitates the backend
initializing LC_MESSAGES before this check. Second, every frontend may
need pg_logging_init() eventually, so it's fair to add calls where missing.

2. Call in set_pglocale_pgservice() and the three programs that don't call
set_pglocale_pgservice(): oid2name, vacuumlo, pgbench.

Thanks,
nm

Attachment Content-Type Size
bestfit05-postmaster-test-basic-v1.patch text/plain 17.0 KB
bestfit10-cmdline-fatal-v2.patch text/plain 14.2 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2024-12-15 02:32:21 Windows UTF8 system locale
Previous Message Amit Kapila 2024-12-15 01:38:44 Re: Conflict detection for update_deleted in logical replication