Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitbd65cb3

Browse files
pg_logicalinspect: Fix possible crash when passing a directory path.
Previously, pg_logicalinspect functions were too trusting of theirinput and blindly passed it to SnapBuildRestoreSnapshot(). If theinput pointed to a directory, the server could a PANIC error whileattempting to fsync_fname() with isdir=false on a directory.This commit adds validation checks for input filenames and passes theLSN extracted from the filename to SnapBuildRestoreSnapshot() insteadof the filename itself. It also adds regression tests for variousinput patterns and permission checks.Bug: #18828Reported-by: Robins Tharakan <tharakan@gmail.com>Co-authored-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>Co-authored-by: Masahiko Sawada <sawada.mshk@gmail.com>Discussion:https://postgr.es/m/18828-0f4701c635064211@postgresql.org
1 parenta49927f commitbd65cb3

File tree

6 files changed

+183
-18
lines changed

6 files changed

+183
-18
lines changed

‎contrib/pg_logicalinspect/Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ DATA = pg_logicalinspect--1.0.sql
1111

1212
EXTRA_INSTALL = contrib/test_decoding
1313

14+
REGRESS = pg_logicalinspect
1415
ISOLATION = logical_inspect
1516

1617
ISOLATION_OPTS = --temp-config$(top_srcdir)/contrib/pg_logicalinspect/logicalinspect.conf
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
CREATE EXTENSION pg_logicalinspect;
2+
-- ===================================================================
3+
-- Tests for input validation
4+
-- ===================================================================
5+
SELECT pg_get_logical_snapshot_info('0-40796E18.foo');
6+
ERROR: invalid snapshot file name "0-40796E18.foo"
7+
SELECT pg_get_logical_snapshot_info('0--40796E18.snap');
8+
ERROR: invalid snapshot file name "0--40796E18.snap"
9+
SELECT pg_get_logical_snapshot_info('-1--40796E18.snap');
10+
ERROR: invalid snapshot file name "-1--40796E18.snap"
11+
SELECT pg_get_logical_snapshot_info('0/40796E18.foo.snap');
12+
ERROR: invalid snapshot file name "0/40796E18.foo.snap"
13+
SELECT pg_get_logical_snapshot_info('0/40796E18.snap');
14+
ERROR: invalid snapshot file name "0/40796E18.snap"
15+
SELECT pg_get_logical_snapshot_info('');
16+
ERROR: invalid snapshot file name ""
17+
SELECT pg_get_logical_snapshot_info(NULL);
18+
pg_get_logical_snapshot_info
19+
------------------------------
20+
21+
(1 row)
22+
23+
SELECT pg_get_logical_snapshot_info('../snapshots');
24+
ERROR: invalid snapshot file name "../snapshots"
25+
SELECT pg_get_logical_snapshot_info('../snapshots/0-40796E18.snap');
26+
ERROR: invalid snapshot file name "../snapshots/0-40796E18.snap"
27+
SELECT pg_get_logical_snapshot_meta('0-40796E18.foo');
28+
ERROR: invalid snapshot file name "0-40796E18.foo"
29+
SELECT pg_get_logical_snapshot_meta('0-40796E18.foo.snap');
30+
ERROR: invalid snapshot file name "0-40796E18.foo.snap"
31+
SELECT pg_get_logical_snapshot_meta('0/40796E18.snap');
32+
ERROR: invalid snapshot file name "0/40796E18.snap"
33+
SELECT pg_get_logical_snapshot_meta('');
34+
ERROR: invalid snapshot file name ""
35+
SELECT pg_get_logical_snapshot_meta(NULL);
36+
pg_get_logical_snapshot_meta
37+
------------------------------
38+
39+
(1 row)
40+
41+
SELECT pg_get_logical_snapshot_meta('../snapshots');
42+
ERROR: invalid snapshot file name "../snapshots"
43+
-- ===================================================================
44+
-- Tests for permissions
45+
-- ===================================================================
46+
CREATE ROLE regress_pg_logicalinspect;
47+
SELECT has_function_privilege('regress_pg_logicalinspect',
48+
'pg_get_logical_snapshot_info(text)', 'EXECUTE'); -- no
49+
has_function_privilege
50+
------------------------
51+
f
52+
(1 row)
53+
54+
SELECT has_function_privilege('regress_pg_logicalinspect',
55+
'pg_get_logical_snapshot_meta(text)', 'EXECUTE'); -- no
56+
has_function_privilege
57+
------------------------
58+
f
59+
(1 row)
60+
61+
-- Functions accessible by users with role pg_read_server_files.
62+
GRANT pg_read_server_files TO regress_pg_logicalinspect;
63+
SELECT has_function_privilege('regress_pg_logicalinspect',
64+
'pg_get_logical_snapshot_info(text)', 'EXECUTE'); -- yes
65+
has_function_privilege
66+
------------------------
67+
t
68+
(1 row)
69+
70+
SELECT has_function_privilege('regress_pg_logicalinspect',
71+
'pg_get_logical_snapshot_meta(text)', 'EXECUTE'); -- yes
72+
has_function_privilege
73+
------------------------
74+
t
75+
(1 row)
76+
77+
-- ===================================================================
78+
-- Clean up
79+
-- ===================================================================
80+
DROP ROLE regress_pg_logicalinspect;
81+
DROP EXTENSION pg_logicalinspect;

‎contrib/pg_logicalinspect/pg_logicalinspect.c

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,45 @@ get_snapbuild_state_desc(SnapBuildState state)
4848
returnstateDesc;
4949
}
5050

51+
/*
52+
* Extract the LSN from the given serialized snapshot file name.
53+
*/
54+
staticXLogRecPtr
55+
parse_snapshot_filename(constchar*filename)
56+
{
57+
uint32hi;
58+
uint32lo;
59+
XLogRecPtrlsn;
60+
chartmpfname[MAXPGPATH];
61+
62+
/*
63+
* Extract the values to build the LSN.
64+
*
65+
* Note: Including ".snap" doesn't mean that sscanf() also does the format
66+
* check including the suffix. The subsequent check validates if the given
67+
* filename has the expected suffix.
68+
*/
69+
if (sscanf(filename,"%X-%X.snap",&hi,&lo)!=2)
70+
gotoparse_error;
71+
72+
/*
73+
* Bring back the extracted LSN to the snapshot file format and compare it
74+
* to the given filename. This check strictly checks if the given filename
75+
* follows the format of the snapshot filename.
76+
*/
77+
sprintf(tmpfname,"%X-%X.snap",hi,lo);
78+
if (strcmp(tmpfname,filename)!=0)
79+
gotoparse_error;
80+
81+
lsn= ((uint64)hi) <<32 |lo;
82+
83+
returnlsn;
84+
85+
parse_error:
86+
ereport(ERROR,
87+
errmsg("invalid snapshot file name \"%s\"",filename));
88+
}
89+
5190
/*
5291
* Retrieve the logical snapshot file metadata.
5392
*/
@@ -60,20 +99,18 @@ pg_get_logical_snapshot_meta(PG_FUNCTION_ARGS)
6099
Datumvalues[PG_GET_LOGICAL_SNAPSHOT_META_COLS]= {0};
61100
boolnulls[PG_GET_LOGICAL_SNAPSHOT_META_COLS]= {0};
62101
TupleDesctupdesc;
63-
charpath[MAXPGPATH];
102+
XLogRecPtrlsn;
64103
inti=0;
65104
text*filename_t=PG_GETARG_TEXT_PP(0);
66105

67106
/* Build a tuple descriptor for our result type */
68107
if (get_call_result_type(fcinfo,NULL,&tupdesc)!=TYPEFUNC_COMPOSITE)
69108
elog(ERROR,"return type must be a row type");
70109

71-
sprintf(path,"%s/%s",
72-
PG_LOGICAL_SNAPSHOTS_DIR,
73-
text_to_cstring(filename_t));
110+
lsn=parse_snapshot_filename(text_to_cstring(filename_t));
74111

75112
/* Validate and restore the snapshot to 'ondisk' */
76-
SnapBuildRestoreSnapshot(&ondisk,path,CurrentMemoryContext, false);
113+
SnapBuildRestoreSnapshot(&ondisk,lsn,CurrentMemoryContext, false);
77114

78115
values[i++]=UInt32GetDatum(ondisk.magic);
79116
values[i++]=Int64GetDatum((int64)ondisk.checksum);
@@ -97,20 +134,18 @@ pg_get_logical_snapshot_info(PG_FUNCTION_ARGS)
97134
Datumvalues[PG_GET_LOGICAL_SNAPSHOT_INFO_COLS]= {0};
98135
boolnulls[PG_GET_LOGICAL_SNAPSHOT_INFO_COLS]= {0};
99136
TupleDesctupdesc;
100-
charpath[MAXPGPATH];
137+
XLogRecPtrlsn;
101138
inti=0;
102139
text*filename_t=PG_GETARG_TEXT_PP(0);
103140

104141
/* Build a tuple descriptor for our result type */
105142
if (get_call_result_type(fcinfo,NULL,&tupdesc)!=TYPEFUNC_COMPOSITE)
106143
elog(ERROR,"return type must be a row type");
107144

108-
sprintf(path,"%s/%s",
109-
PG_LOGICAL_SNAPSHOTS_DIR,
110-
text_to_cstring(filename_t));
145+
lsn=parse_snapshot_filename(text_to_cstring(filename_t));
111146

112147
/* Validate and restore the snapshot to 'ondisk' */
113-
SnapBuildRestoreSnapshot(&ondisk,path,CurrentMemoryContext, false);
148+
SnapBuildRestoreSnapshot(&ondisk,lsn,CurrentMemoryContext, false);
114149

115150
values[i++]=CStringGetTextDatum(get_snapbuild_state_desc(ondisk.builder.state));
116151
values[i++]=TransactionIdGetDatum(ondisk.builder.xmin);
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
CREATE EXTENSION pg_logicalinspect;
2+
3+
-- ===================================================================
4+
-- Tests for input validation
5+
-- ===================================================================
6+
7+
SELECT pg_get_logical_snapshot_info('0-40796E18.foo');
8+
SELECT pg_get_logical_snapshot_info('0--40796E18.snap');
9+
SELECT pg_get_logical_snapshot_info('-1--40796E18.snap');
10+
SELECT pg_get_logical_snapshot_info('0/40796E18.foo.snap');
11+
SELECT pg_get_logical_snapshot_info('0/40796E18.snap');
12+
SELECT pg_get_logical_snapshot_info('');
13+
SELECT pg_get_logical_snapshot_info(NULL);
14+
SELECT pg_get_logical_snapshot_info('../snapshots');
15+
SELECT pg_get_logical_snapshot_info('../snapshots/0-40796E18.snap');
16+
17+
SELECT pg_get_logical_snapshot_meta('0-40796E18.foo');
18+
SELECT pg_get_logical_snapshot_meta('0-40796E18.foo.snap');
19+
SELECT pg_get_logical_snapshot_meta('0/40796E18.snap');
20+
SELECT pg_get_logical_snapshot_meta('');
21+
SELECT pg_get_logical_snapshot_meta(NULL);
22+
SELECT pg_get_logical_snapshot_meta('../snapshots');
23+
24+
-- ===================================================================
25+
-- Tests for permissions
26+
-- ===================================================================
27+
CREATE ROLE regress_pg_logicalinspect;
28+
29+
SELECT has_function_privilege('regress_pg_logicalinspect',
30+
'pg_get_logical_snapshot_info(text)','EXECUTE');-- no
31+
SELECT has_function_privilege('regress_pg_logicalinspect',
32+
'pg_get_logical_snapshot_meta(text)','EXECUTE');-- no
33+
34+
-- Functions accessible by users with role pg_read_server_files.
35+
GRANT pg_read_server_files TO regress_pg_logicalinspect;
36+
37+
SELECT has_function_privilege('regress_pg_logicalinspect',
38+
'pg_get_logical_snapshot_info(text)','EXECUTE');-- yes
39+
SELECT has_function_privilege('regress_pg_logicalinspect',
40+
'pg_get_logical_snapshot_meta(text)','EXECUTE');-- yes
41+
42+
-- ===================================================================
43+
-- Clean up
44+
-- ===================================================================
45+
46+
DROP ROLE regress_pg_logicalinspect;
47+
48+
DROP EXTENSION pg_logicalinspect;

‎src/backend/replication/logical/snapbuild.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1691,12 +1691,17 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
16911691
* If 'missing_ok' is true, will not throw an error if the file is not found.
16921692
*/
16931693
bool
1694-
SnapBuildRestoreSnapshot(SnapBuildOnDisk*ondisk,constchar*path,
1694+
SnapBuildRestoreSnapshot(SnapBuildOnDisk*ondisk,XLogRecPtrlsn,
16951695
MemoryContextcontext,boolmissing_ok)
16961696
{
16971697
intfd;
16981698
pg_crc32cchecksum;
16991699
Sizesz;
1700+
charpath[MAXPGPATH];
1701+
1702+
sprintf(path,"%s/%X-%X.snap",
1703+
PG_LOGICAL_SNAPSHOTS_DIR,
1704+
LSN_FORMAT_ARGS(lsn));
17001705

17011706
fd=OpenTransientFile(path,O_RDONLY |PG_BINARY);
17021707

@@ -1788,18 +1793,13 @@ static bool
17881793
SnapBuildRestore(SnapBuild*builder,XLogRecPtrlsn)
17891794
{
17901795
SnapBuildOnDiskondisk;
1791-
charpath[MAXPGPATH];
17921796

17931797
/* no point in loading a snapshot if we're already there */
17941798
if (builder->state==SNAPBUILD_CONSISTENT)
17951799
return false;
17961800

1797-
sprintf(path,"%s/%X-%X.snap",
1798-
PG_LOGICAL_SNAPSHOTS_DIR,
1799-
LSN_FORMAT_ARGS(lsn));
1800-
18011801
/* validate and restore the snapshot to 'ondisk' */
1802-
if (!SnapBuildRestoreSnapshot(&ondisk,path,builder->context, true))
1802+
if (!SnapBuildRestoreSnapshot(&ondisk,lsn,builder->context, true))
18031803
return false;
18041804

18051805
/*

‎src/include/replication/snapbuild_internal.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ typedef struct SnapBuildOnDisk
193193
/* variable amount of TransactionIds follows */
194194
}SnapBuildOnDisk;
195195

196-
externboolSnapBuildRestoreSnapshot(SnapBuildOnDisk*ondisk,constchar*path,
196+
externboolSnapBuildRestoreSnapshot(SnapBuildOnDisk*ondisk,XLogRecPtrlsn,
197197
MemoryContextcontext,boolmissing_ok);
198198

199199
#endif/* SNAPBUILD_INTERNAL_H */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp