JavaScript: @tanstack/react-query, @tanstack/react-table, dompurify,
lossless-json, rc-dock, react, react-dom, react-dropzone,
react-virtualized-auto-sizer, html-react-parser, zustand,
@mui/x-date-pickers, core-js, webpack, postcss, typescript-eslint,
eslint-plugin-jest, globals, @testing-library/user-event.
The postcss resolution override is raised along with the direct
dependency, so installs actually resolve to 8.5.31.
Python (python_version > '3.9' rows only): Authlib, certifi,
Flask-SocketIO, urllib3 and selenium.
Held back: @tanstack/react-virtual (3.14.13) and marked (18.0.14) break
the Jest suite in later releases, and pickr stays at ~1.9.1 because
1.10.x breaks the production bundle.
Feature tests:
- Backup and Restore clicked their Tools menu item while it was still
disabled, because the menus are refreshed asynchronously after a
database gets connected. Wait for the item to be enabled first.
- check_if_element_exists_with_scroll() never terminated when it reached
the bottom of the tree and the node it looked for was above the visible
area. Stop scrolling down at that point.
add-user and add-external-user defaulted --role to None, so without
--role or --admin they looked up a role named None and exited with
"Role 'None' does not exists." Default --role to 'User', the role
pgAdmin gives every user it creates itself. --admin still takes
precedence, and an unknown --role still fails.
The 'LDAP Authentication with Auto Create User' scenario logged in
ldap_user, creating an LDAP account that was never deleted, and the
'without Auto Create User' scenario only passed because that account was
already there; run on its own it failed.
The 'without' scenario now adds the LDAP account it logs in to with
add_user(), which deletes it with the rows the login created when the
test ends, and every auto-create scenario deletes the account its login
created.
After adding a user, setup.py looked the new account up for display by
its email together with the auth source. For an external user the email
is not the username, so without --email every user of that auth source
was listed, and with --email nothing was shown at all.
Look the account up by the username it was created with and its auth
source instead, and add tests that run setup.py add-user and
add-external-user against a copy of the test database and check that
only the new account is shown.
- Webserver login and auto-creation looked the account up by username
alone. An internal account with the same username was found first, so
the Webserver user was refused, or never created.
- LDAP auto-creation, when LDAP_DN_CASE_SENSITIVE is off, compared the
stored username with the lower-cased login name, so an LDAP user whose
username has capitals was not found after their first login, and the
attempt to create them again failed, so they could not log in. It now
compares both lower-cased.
- setup.py update-user and update-external-user failed with a KeyError
unless --role or --admin was given, so an account could not be
deactivated on its own, as the LDAP documentation describes.
A test also checks that a password reset requested with the email in
another case is sent for the internal account.
Flask-Security registers views of its own next to pgAdmin's: /login,
/verify, /change, /reset and /reset/<token>. Submitting to them logged a
user in without the failed login count or the lock, checked a signed-in
user's password without limit, changed a password without pgAdmin's
change password permission or re-encrypting the saved server passwords,
and sent or applied a password reset for an account that is not
internal or is locked. pgAdmin does all of these through views of its
own, so a submission to Flask-Security's is now refused with 405
Method Not Allowed. A GET still renders the page, as GET /login renders
pgAdmin's login page, whose form is posted to /authenticate/login. The
refusal is not logged as an error, as anyone can send one.
The test uses server mode only views and is excluded in desktop mode.
The limit on failed logins (MAX_LOGIN_ATTEMPTS) could be got round, and
its lock reached accounts it does not protect:
- The internal account was looked up by its exact username, while
Flask-Security matches the email ignoring case, so a username typed in
another case was neither counted nor checked for the lock. It is now
looked up as Flask-Security does.
- With LDAP or Webserver authentication also configured, the login form
always passed validation, so a wrong internal password was never
counted. It is now counted whenever no source authenticates the user
and internal authentication was tried.
- A login that posts a password no longer falls back to Kerberos, OAuth2
or Webserver authentication, which do not check it: with them listed,
a guess was answered by the Kerberos challenge, or silently logged in
the user the web server named, without being counted.
- The account is locked on the failed login that reaches the limit, and
the user is told so, rather than at the next login.
- A login through another source no longer resets the internal
account's failed logins.
The lock is the internal account's. A locked internal account only stops
internal authentication: the other sources are still tried and only ever
log in an account of their own, so a user of another source with the
same name is not locked out by it. The password is not checked against
the locked account. When another source refuses the user, one neutral
message is shown whatever the password, saying a pgAdmin account may be
locked; with no other source to try, the account locked message is shown
as before. The skip is logged.
The new tests use server mode only features and are excluded in desktop
mode. The Kerberos login test now posts no password, as a Kerberos login
does.
The same email or username may belong to accounts from several auth
sources. Flask-Security looks the account up by email alone, and took
whichever row came first:
- Internal login checked the password against that row, so an internal
user could not log in when an LDAP or OAuth2 account with the same
email was found first. Flask-Security's lookup now prefers the internal
account; ignoring case, it prefers the one matching exactly, then the
oldest. A missing email or username identifies no account.
- A password reset was sent for whichever account had the email, and a
reset token for an account from another source was accepted, giving it
a local password and logging it in. Reset instructions are now sent
for the internal account, and a token for any other account is
refused.
- The About box showed the configuration to any user whose email was
that of an administrator's account, whatever its source. It now checks
the user's own role.
The test runner excluded every test module whose name started with one
on its desktop mode list, so excluding test_reset_password also excluded
test_reset_password_auth_source. Each entry now excludes only the module
it names, and the server mode only modules are listed.
In desktop mode get_storage_directory() returns None and a path is
absolute, so check_access_permission() returns it unchanged. These tests
patch in a storage directory in either mode and passed a path relative
to it, which in desktop mode named a file at the root of the filesystem,
so the save failed. They now pass a path inside the storage directory in
desktop mode, as a desktop user would.
The tests patched the application's SERVER_MODE setting but not
config.SERVER_MODE, which get_locale() reads. In desktop mode the refusal
message's gettext() then took the desktop branch and read
current_user.is_authenticated from the stand-in user, which has no such
attribute, so the "non-admin cannot write" scenario failed. Patch
config.SERVER_MODE as well in the three tests that use this setup.
In server mode, check_access_permission() validated a path into the
user's storage directory, but several callers then acted on a path built
differently, so the path checked and the path used could name different
files:
- Opening a file in the Query Tool, saving a file, the Google client
secret and, on Windows, Import/Export Data, Backup and Restore joined
the path to the storage directory after stripping leading separators,
so a path checked as inside storage could name any file the pgAdmin
account can read, or on Windows write outside it.
- Listing a folder decoded the path again after the check, so a folder
named like an encoded ".." listed any directory on the host.
- Download, delete, rename and the file-exists check appended the path
to the storage directory without a separator, so where one user's
storage directory name was a prefix of another's, that user's files
could be read, deleted or moved; download also checked the path
appended to itself.
- Creating a folder checked the parent rather than the folder created,
and upload wrote to a path that was not checked.
check_access_permission() now returns the absolute path it checked, and
every caller acts on that path. Folder paths are decoded once, rename
checks the new name where it is created, and download returns a 404
without the server path for a missing file.
In server mode on POSIX, download, delete, rename and file loads now
open files relative to directories opened from the storage directory
without following symbolic links, as upload and save already did, so a
parent directory swapped for a symbolic link after the check is refused.
The walk follows the path as written: a ".." goes back up past a
directory the walk opened, rather than being collapsed in the text to a
different file from the one the check resolved. It keeps open only the
directories a later ".." can still go back up to, so a path holds at
most two more file descriptors than it has ".." segments, and working
out which to keep takes time linear in the length of the path.
Also:
- The storage directory itself can no longer be deleted.
- In desktop mode, a download refuses a FIFO or other non-regular file
instead of blocking on it; symbolic links are still followed there.
- A symbolic link refusal is recognised as ELOOP, or EMLINK only where
O_NOFOLLOW uses it.
- The File Manager sends the selected storage when checking whether a
file exists, so the overwrite prompt sees files in a shared storage.
- get_closest_parent() tested whether the storage directory existed
rather than the remembered folder; it now finds the closest existing
folder inside storage.
- A refused or failed download now shows the server's message.
- check_file_for_bom_and_binary() reports a read error in its result.
The same username may exist under several auth sources. Kerberos uses
the shared login(), which looked the account up by username alone, so a
Kerberos principal matching an internal account's username was logged in
as that internal account, including the initial administrator. login()
now looks the account up by the authenticated username and the source
that authenticated it.
form.user is no longer used by any source but internal. Flask-Security's
validation, run for internal authentication, sets it to the account whose
email was typed, whether or not its password matched. With internal
authentication listed first, a Kerberos user could type another Kerberos
user's principal and be logged in as them, and an LDAP user could do the
same with an LDAP account whose email is their username. Internal login
logs in form.user, whose password was verified, only if it is an
internal account, and otherwise nothing.
The LDAP + Internal login test relied on an LDAP bind that accepted the
internal test account's credentials logging in that internal account,
which is the behaviour fixed here. LDAP now refuses those credentials in
that scenario, and new scenarios check that an LDAP login whose username
matches an internal account is not logged in as it, with LDAP or
internal authentication tried first. A further test logs in through the
login endpoint as a Kerberos user who types another Kerberos user's
principal. The LDAP documentation's add-external-user example passed the
source as an argument; it is given with --auth-source.
LDAPAuthentication.login() looked up the user to log in by username
alone, or accepted the form.user set by Flask-Security's internal
validation, without checking its auth source. Because the same username
may exist under several auth sources, an LDAP user whose username matched
an Internal account (such as the initial administrator created at setup)
was logged in as that Internal account, with its roles.
Only use form.user if it is LDAP-sourced, and restrict the fallback
lookups to auth_source=LDAP, matching the existing guard in Webserver
authentication.
Document that the initial administrator is always an Internal account,
and recommend deactivating it in favour of an LDAP administrator when
using LDAP authentication only.
Reported-by: Yves Soete of Blacksight LLC
sshtunnel was given neither host_pkey_directories nor ssh_config_file, so it
always loaded the unencrypted private keys in ~/.ssh of the account pgAdmin
runs as, and read that account's ~/.ssh/config (IdentityFile, User, HostName,
ProxyCommand) for the tunnel host. In server mode any user could therefore
configure a tunnel to a host reachable by that account, with no credentials
or an identity file that does not exist in their storage, and authenticate
with the service account's key. The ssh-agent of that account could also be
used.
In server mode the tunnel no longer reads those keys or that configuration,
and does not use the agent. An identity file that cannot be found in the
user's storage is reported instead of silently falling back to ~/.ssh.
Desktop mode is unchanged.
Compatibility: in server mode, SSH tunnels that relied on the service
account's own key, ~/.ssh/config or ssh-agent will stop working; the
identity file or password must be configured on the server instead.
Rotate fs_uniquifier on a password change (self-service and admin reset), so
every other session of that user is logged out. The changing session is
re-logged in, and its database connections are kept.
Re-encrypt saved passwords, including shared-server ones, under the new key.
Passwords that can't be decrypted are cleared, and the user is told. If
re-encryption fails, the sessions are still revoked and all saved passwords
are cleared rather than returning an error.
Reported-by: Zohaib
does_utility_exist() and /misc/validate_binary_path only rejected a
binary path under STORAGE_DIR, so a user could point the binary path of
a server type at a SHARED_STORAGE directory that ordinary users can
write to, and have pgAdmin execute whatever file was placed there
(CWE-426).
The check is now made by is_reserved_storage_path(), shared by both call
sites and extended to every SHARED_STORAGE entry. It expands $DIR and
collapses '..' before resolving symlinks, so it judges the directory the
utilities actually run from, compares directories with samefile() so
that a different spelling on a case-insensitive filesystem cannot get
past it, and refuses a path containing a NUL. An empty STORAGE_DIR is
treated as unreserved instead of being resolved against the working
directory. The guard is enforced inside get_binary_path_versions(), so
no caller can skip it, and the PSQL tool now goes through
does_utility_exist() too.
Reported-by: Harsh Singh
_is_query_parens_balanced() rejected a backslash inside a single-quoted
string but not inside a double-quoted identifier, where it treated the
next quote as closing the token. psql does not: strtokx in
parse_slash_copy takes its escape character from the connection's
standard_conforming_strings setting and applies it to whichever quote
opened the token, so with standard_conforming_strings off the \" in
"a\"b" is an escaped quote and the identifier runs on to the next quote.
That desynchronises our quote state from psql's in the opposite
direction to the case fixed in 1496fabe2, and a query export such as
SELECT "a\"b"c) TO PROGRAM 'cmd' x" therefore passed the balance check
whilst the ) it hid closed the wrapping \copy (...) context, exposing a
live TO PROGRAM clause. psql calls popen() for a client-side
\copy ... TO PROGRAM before the backend sees the statement at all, so
the command runs on the pgAdmin host even though the query is then
rejected as a syntax error. An ordinary PostgreSQL role can set
standard_conforming_strings on itself, and a non-administrator pgAdmin
account holds tools_import_export_data by default, so no privileged
access of either kind is needed.
Apply the same treatment as the single-quoted branch and reject any
backslash encountered whilst scanning a double-quoted identifier, since
the correct interpretation still depends on a server setting we cannot
reliably know at validation time.
Co-authored-by: Dave Page <dpage@pgadmin.org>
Reported-by: Yonghwa Lee of Xint by Theori
When a connection was not meant to use a Kerberos user's delegated
credentials, pgAdmin removed KRB5CCNAME from its environment, so the
Kerberos library fell back to the default credential cache of the account
pgAdmin runs as. libpq then answered any server that asked for GSSAPI
authentication with that account's ticket, whatever user name the
pgAdmin user gave, so any user could connect as the service account's
principal; gssencmode=disable does not prevent this. A client keytab for
that account had the same effect, even with no cache, as the library
fills the cache from it. On Windows, SSPI used the service account's
logon in the same way. psql was also started with a copy of pgAdmin's
environment, which could hold another user's credential cache whilst that
user was connecting, and the connection made to list a publisher's
publications was not made under the lock that guards it.
In server mode KRB5CCNAME and KRB5_CLIENT_KTNAME now always point at a
file that cannot exist, except whilst a user who logged in with Kerberos
connects to a server with Kerberos authentication enabled, when
KRB5CCNAME is that user's delegated cache as before. Because environment
isolation alone does not stop SSPI or gssproxy, every other connection
also gets require_auth=!gss,!sspi and gssencmode=disable, merged with the
server's own require_auth, which is refused if it allowed only GSS or
SSPI. A Kerberos-allowed connection forces gsslib=gssapi, so libpq uses
the delegated cache rather than the Windows logon. psql and the utilities
receive the same settings as PGREQUIREAUTH, PGGSSENCMODE and PGGSSLIB;
background processes now pass the server's own kerberos_conn instead of
always assuming a Kerberos connection, and the publication lookup
connects under the lock. Desktop mode is unchanged.
The forced require_auth, gssencmode and gsslib are serialised last, so
they win over any value a crafted connection parameter key places before
them, and the final connection string is parsed and refused if any of
them differs from the value forced (or is present where it is left to
the environment), so a smuggled value cannot steer the PSQL tool or a
utility to GSSAPI or SSPI however the parameters were serialised.
A pg_service.conf entry ranks above the PG* environment variables, so
only a service the administrator lists in SERVER_MODE_ALLOWED_SERVICES
can override these settings for the utilities; the documentation says
such an entry must not set them.
Compatibility: in server mode, connecting with the service account's
Kerberos ticket, client keytab or Windows logon (SSPI) no longer works;
only users who log in to pgAdmin with Kerberos can use GSSAPI, with their
own delegated credentials, as the documentation already described. A
server whose require_auth lists only gss or sspi is refused unless the
user logged in with Kerberos (SSPI-only is never usable). require_auth
needs libpq 16 or later; with an older libpq only the KRB5CCNAME and
KRB5_CLIENT_KTNAME isolation applies, which does not cover Windows SSPI
or gssproxy.
Co-authored-by: Dave Page <dpage@pgadmin.org>
Co-authored-by: hiteshjambhale <hitesh.jambhale@enterprisedb.com>
libpq looks a service up in the connection service file of the account
pgAdmin runs as, and an entry there can hold a password or name a
password file or client certificate. In server mode any user could set a
server's service, or a 'service' connection parameter, to one of those
entries and connect with its credentials. Likewise, a PGPASSWORD or
PGSERVICE variable set for that account was used by libpq for every
connection that had no value of its own, and passed on to the utilities.
In server mode a service may now only be used if it is listed in the new
SERVER_MODE_ALLOWED_SERVICES setting, which is empty by default. Creating
or updating a server, an ad hoc workspace connection or a servers.json
import that names any other service is refused, and connecting to an
existing server, or running psql or a utility for it, fails with a
message that names the setting. The 'servicefile' connection parameter is
refused in server mode, and PGPASSWORD and PGSERVICE are removed from
pgAdmin's environment at startup, with a warning. PGSERVICEFILE and
PGSYSCONFDIR are kept, since they locate the listed services. Desktop
mode is unchanged.
The allowlist is checked again on the final connection string, as libpq
will parse it, so a service smuggled in through a crafted parameter key
is refused however the parameters were serialised. pg_dumpall expands a
--database value containing "=" as a connection string, so the
maintenance database name of Backup Server and Globals is wrapped in one
and a name such as "service=name" no longer selects a service from the
service account's service file. client_files_isolated() treats a missing
PGADMIN_RUNTIME as "not the desktop runtime", so "setup.py load-servers"
no longer fails on any servers.json containing a Service and still
enforces the allowlist in server mode.
The tests run the real job-creation path of every tool that starts a
utility, and check that a service that is not listed is refused before
the utility starts, a listed one is exported as PGSERVICE, and a service
or servicefile connection parameter, or a database name that looks like
a connection string, never selects a service.
Compatibility: in server mode, servers that use a service stop
connecting until the administrator lists it in
SERVER_MODE_ALLOWED_SERVICES, and a PGPASSWORD or PGSERVICE set in the
environment pgAdmin starts with is ignored. "setup.py load-servers" now
refuses a servers.json whose Service is not in
SERVER_MODE_ALLOWED_SERVICES (exit status 1) instead of printing an
error and exiting 0.
Co-authored-by: Dave Page <dpage@pgadmin.org>
Co-authored-by: hiteshjambhale <hitesh.jambhale@enterprisedb.com>
To pass a server's password to psql and to the utilities run as background
processes, ServerManager.export_password_env() wrote it into os.environ:
as PGPASSWORD for psql, and under the process ID for a utility. Nothing
removed it again, and pgAdmin's environment is shared by every user's
requests and copied into every process it starts, so another user's later
psql session or utility inherited the password, and libpq read PGPASSWORD
for any connection made without one of its own. In server mode that let
one user connect with another user's saved password.
The password now goes only into the environment built for the process
that needs it: export_password_env() fills in a dict it is given, the
background process adds it to the environment of that process alone, and
psql is started with a copy of pgAdmin's environment plus its password,
rather than with os.environ itself, which get_user_env() also modified by
setting the user's psqlrc and history paths in it. The process executor
removes the variable it reads the password from, so the utility only
sees PGPASSWORD.
There is no compatibility impact: psql and the utilities receive the same
password in the same variable as before.
Co-authored-by: Dave Page <dpage@pgadmin.org>
When a server had no password file, client certificate or client key
configured, or the configured one could not be found, pgAdmin left the
parameter out of the connection and libpq fell back to ~/.pgpass,
~/.postgresql/postgresql.crt and ~/.postgresql/postgresql.key (or
PGPASSFILE, PGSSLCERT and PGSSLKEY) belonging to the account pgAdmin runs
as. In server mode that let any user connect with the service account's
saved passwords or client certificate, and supplying a copy of its public
certificate was enough for libpq to pair it with the service account's
private key.
In server mode each of these parameters is now always given a value:
the user's own file if one is configured, otherwise a path that can never
be opened, so libpq neither uses nor looks for a default. This applies to
the connection string, to the PGPASSFILE, PGSSLCERT and PGSSLKEY variables
passed to the utilities, and to the direct connection made to list a
publisher's publications. A configured file that does not exist is now
passed on as configured instead of being dropped, so a missing root
certificate or CRL no longer quietly falls back to root.crt or root.crl,
and a CRL directory is now passed on in server mode. The trust settings
(sslrootcert, sslcrl, sslcrldir) are still left to libpq when they are not
configured, as they hold no secret, and "system" is no longer mistaken for
a file name in the subscription connection. Desktop mode is unchanged.
libpq ranks a pg_service.conf entry above the PG* environment variables,
so a service entry that set passfile, sslcert or sslkey still won for
pg_dump, pg_restore, psql and the maintenance tools. In server mode the
three settings are therefore also passed in the --dbname connection
string, which libpq ranks above the service file; pg_dumpall takes them
in a separate --dbname argument so that its per-database pg_dump
processes inherit them. pg_dumpall's --database, the maintenance database
name passed raw, is now wrapped as a connection string too, so a name
such as "passfile=..." cannot bring the service account's files back. The
file settings are chosen from the same source as the environment
variables, so that the two cannot disagree.
Compatibility: server mode deployments that relied on the service
account's ~/.pgpass, PGPASSFILE, default client certificate and key,
PGSSLCERT or PGSSLKEY, or on a passfile, sslcert or sslkey in a
pg_service.conf entry, will need to configure those files on each server
instead. In server mode a pg_dump, pg_restore or psql older than
PostgreSQL 10 fails on the passfile keyword, and a server configured with
an sslcert but no sslkey now fails with "certificate present, but not
private key file" instead of silently using the service account's key.
The container is not affected, as its entrypoint already copies
PGPASS_FILE into the user's storage and sets the password file
explicitly. Passwords in a pg_service.conf entry chosen through the
service setting are not addressed here.
The tests now cover a server whose user password file is held only on
the server row. The auth gating test restores the provider modules it
re-imports, so later OAuth2 tests no longer fail depending on test
order.
Co-authored-by: Dave Page <dpage@pgadmin.org>
The server dialog only shows the Shared switch to administrators, and
the servers.json import only honoured it for them, but the create and
update endpoints stored whatever 'shared' and 'shared_username' they
were sent. Any user able to register a server could therefore share one
with every other user of a server mode installation. The import had a
similar gap: validate_json_data() removed a non-administrator's "Shared"
entries only after its loop, so an early validation error left them in
the data, and the web path ignored that error, so selecting a shared
entry in a file that also held an invalid one created a shared server.
A non-administrator's request that would share a server is now refused
with a 403. A 'shared' value that changes nothing is still accepted, so
editing a private server is unaffected, and 'shared_username' is dropped
rather than refused because the dialog sends it whenever the username
changes. The import prunes a non-administrator's shared entries before
validating and skips them again for each imported entry, only a JSON
true counts as shared, and validation errors are reported on the web
path too. Copy Server no longer sends shared for a non-administrator
owner.
The existing servers are cleared for "replace" only after the file has
validated, so a refused file no longer deletes them. Servers shared by
non-administrators before the upgrade stay shared until the owner
un-shares them or an administrator changes them; pgAdmin warns about
them at server-mode startup, and the documentation describes this.
Compatibility: a servers.json import in server mode that fails
validation is now refused as a whole on the web path, as it already was
in setup.py.
Co-authored-by: Dave Page <dpage@pgadmin.org>
In server mode, get_complete_file_path() joined the user's storage
directory with the path they supplied after stripping only leading
slashes, so a password file, SSL certificate, key, CRL or SSH tunnel
identity file set to "../<another user>/.pgpass" (through the server
dialog, the API or a servers.json import) would connect with another
user's credentials or key. The sslkeylogfile parameter was copied into
the connection string unchanged, so a traversal path let libpq create or
append a file as the pgAdmin service account.
The resolved path, with symbolic links followed, must now stay inside
the user's storage directory, or inside the named shared storage
directory for a "Name:/file" path, as the file manager already requires.
sslkeylogfile is confined the same way, is refused in a shared storage
directory that is read-only for the user, and is no longer inherited by
non-owners of a shared server. A path that escapes, or whose storage
directory cannot be determined, raises PermissionError rather than
letting libpq fall back to its default file, and the message says where
the file has to live. Desktop mode is unchanged.
make_conninfo() quotes connection-parameter values but not their keys,
so a key such as 'sslmode=require passfile' smuggled its own
keyword=value pairs past these checks. Only exact libpq keywords, read
from the linked libpq at runtime, are now accepted as connection
parameter keys: when a server is created, updated, imported from
servers.json or used for an ad-hoc connection, and again when the
connection string is built, for rows saved earlier.
Connecting, the SSH tunnel and the utilities' job creation report a
refused path or key instead of proceeding, and a job whose environment
cannot be resolved is removed rather than left in the Processes list. A
server with a refused path or key can still be listed and edited to
correct it. On Windows, the short path of a not-yet-existing file is
resolved via its parent directory instead of being dropped.
Co-authored-by: Dave Page <dpage@pgadmin.org>
Co-authored-by: hiteshjambhale <hitesh.jambhale@enterprisedb.com>
Follow-up to the Import/Export Servers, File Manager and utility-file
symlink-race fixes.
- Hard links: O_NOFOLLOW refuses only symbolic links, so a second name
for a file outside the storage directory was still opened.
_open_upload_target(), _open_read_target() and open_utility_file() now
refuse a file whose link count is not 1, reporting a file unlinked
after the open separately from one with several links. For writes,
O_TRUNC is applied only after that check (via ftruncate), so a
hard-linked file is never emptied before it is refused.
- FIFOs on write: _open_upload_target() opens non-blocking and accepts
only a regular file, so a FIFO planted at the path can no longer hang
an upload, save or export waiting for a reader. _open_beneath()
reports the ENXIO of a reader-less FIFO as "Not a regular file", so
Backup, Restore and Import/Export Data get the same message.
- Errors: the Import/Export Servers, save_file, Backup, Restore and
Import/Export Data handlers return the reason (strerror) rather than
str(e), which included the absolute storage path, and fall back to the
exception's text for errors without an errno, so a non-UTF-8
servers.json is reported instead of failing with a 500.
load_database_servers() keeps the file path in its messages only for
the setup.py CLI, and the upload response returns the
storage-relative name instead of the on-disk path.
Tests cover the hard-link refusal (target left intact), reader-less
FIFOs on the upload and utility-write paths, the non-UTF-8 servers.json
and the handlers' path-free errors.
Backup, Restore and Import/Export Data checked the user's file path
against their storage directory with filename_with_file_manager_path()
and then passed the path to pg_dump, pg_dumpall, pg_restore or psql,
which opened it by name later. A symbolic link swapped in at the leaf or
a parent directory in between redirected the utility outside the storage
directory, so a backup could overwrite, or a restore or \copy read,
another file that pgAdmin's OS user can reach.
In server mode pgAdmin now opens the file itself through the File
Manager's descriptor-relative O_NOFOLLOW walk from the storage directory,
accepts only a regular file, and hands the utility /dev/fd/N.
BatchProcess.start() gains pass_fds and launch-only arguments so the
descriptor is inherited by the process executor and the utility, whilst
the job details stored and shown to the user keep the real path.
Descriptors are only passed on Linux; on macOS and the BSDs the file is
still opened and checked safely but the utility reopens it by name
afterwards. Desktop mode and Windows are unchanged.
dump_database_servers() validated the servers.json path against the
user's storage directory with filename_with_file_manager_path(), then
wrote it with a plain open(), so a symbolic link swapped in at the leaf,
or at any parent directory of it, between the check and the write let the
export overwrite a file outside the storage directory.
load_database_servers() and the import file-listing endpoint had the same
gap on the read side.
A new _open_beneath() opens the storage directory and then walks down to
the file one component at a time, each open relative to the descriptor of
the directory above it and made with O_NOFOLLOW, so the kernel refuses a
symbolic link wherever it appears below the storage root. New files are
created 0o600, and reads accept only a regular file, opened non-blocking
so that a FIFO cannot hang the request. Desktop mode, Windows and the
setup.py CLI path are unchanged.
Reported-by: MUTED CYBERSECURITY S.L.
MViewNode.refresh_data() passed the target database's name straight to
psql's --dbname argument. libpq treats a --dbname value containing "="
(or a postgresql:// URI) as a full connection string, so a database
named e.g. 'host=attacker port=5432 dbname=postgres' - creatable by any
role with only CREATEDB - redirects the spawned psql to an attacker host,
leaking the connecting user's saved server password.
Route the name through database_conninfo(), the same helper already used
for backup, restore, maintenance and import/export, so libpq treats it
strictly as a database name and never as connection directions.
Reported-by: Bae Seung-won of Ajou University
pgAdmin renders its templates with trim_blocks, which removes the newline
after a block tag. nodes.sql ended its WHERE clause with a conditional, so
with neither a statistics id nor schema_diff set, which is what expanding
the Statistics collection sends, the SQL ran into "::oidORDER BY" and the
server rejected it.
Give each condition its own line ending in plain text, and add a test that
renders the template for every parameter combination and runs it.
Ashesh Vashi
Dave Page
Kundan
hiteshjambhale
dependabot[bot]