Skip to content

Commit b043ab9

Browse files
authored
Fix crash in tsql_row_to_xml_path function (#4665)
Fixed a crash in tsql_row_to_xml_path function that occurred when processing FOR XML PATH queries with all-null column values. The crash was caused by unsafe string buffer manipulation when converting null elements to self-closing XML tags without proper bounds checking. This fix reorganizes the null-handling logic to prevent buffer underflow. Task: BABEL-5364 Authored-by: Rohit Bhagat rohitbgt@amazon.com
1 parent 67ce5e2 commit b043ab9

File tree

7 files changed

+107
-22
lines changed

7 files changed

+107
-22
lines changed

contrib/babelfishpg_tsql/src/tsql_for/forxml.c

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ tsql_query_to_xml_text_ffunc(PG_FUNCTION_ARGS)
124124
{
125125
StringInfo res = for_xml_ffunc(fcinfo);
126126

127+
/* return NULL if empty result (i.e. no rows) */
128+
if (res->len == 0)
129+
PG_RETURN_NULL();
130+
127131
PG_RETURN_TEXT_P(cstring_to_text_with_len(res->data, res->len));
128132
}
129133

@@ -266,7 +270,7 @@ validate_attribute_centric_col_names_xml(const char *element_name, TupleDesc tup
266270
seen_non_att_centric = true;
267271
}
268272

269-
if(seen_att_centric && element_name[0] == '\0')
273+
if(seen_att_centric && (element_name && strlen(element_name) == 0))
270274
{
271275
ereport(ERROR,
272276
(errcode(ERRCODE_INVALID_XML_PROCESSING_INSTRUCTION),
@@ -284,14 +288,15 @@ static void
284288
tsql_row_to_xml_path(StringInfo state, Datum record, const char *element_name, bool binary_base64)
285289
{
286290
HeapTupleHeader td;
287-
Oid tupType;
288-
int32 tupTypmod;
289-
TupleDesc tupdesc;
290-
HeapTupleData tmptup;
291-
HeapTuple tuple;
292-
bool allnull = true;
293-
bool has_att_centric = false;
294-
bool first = true;
291+
Oid tupType;
292+
int32 tupTypmod;
293+
TupleDesc tupdesc;
294+
HeapTupleData tmptup;
295+
HeapTuple tuple;
296+
bool allnull = true;
297+
bool has_att_centric = false;
298+
bool first = true;
299+
int inital_state_len = state->len;
295300

296301
td = DatumGetHeapTupleHeader(record);
297302

@@ -311,7 +316,7 @@ tsql_row_to_xml_path(StringInfo state, Datum record, const char *element_name, b
311316
* each tuple is either contained in a "row" tag, or standalone if the
312317
* element_name is an empty string
313318
*/
314-
if (element_name[0] != '\0')
319+
if (element_name && strlen(element_name) > 0)
315320
{
316321
/* if "''" is the input path, ignore it per TSQL behavior */
317322
if (has_att_centric)
@@ -372,22 +377,30 @@ tsql_row_to_xml_path(StringInfo state, Datum record, const char *element_name, b
372377
}
373378
}
374379

375-
if (allnull)
376-
{
377-
/*
378-
* If all the column values are nulls, this element should be
379-
* <element_name/>, modify the already appended <element_name> to
380-
* <element_name/>.
381-
*/
382-
state->data[state->len - 1] = '/';
383-
appendStringInfoString(state, ">");
384-
}
385-
else if (element_name[0] != '\0')
380+
if (element_name && strlen(element_name) > 0)
386381
{
387382
if (has_att_centric && first)
388383
appendStringInfoString(state, "/>");
389384
else
390-
appendStringInfo(state, "</%s>", element_name);
385+
{
386+
if (allnull)
387+
{
388+
/*
389+
* At this point, state = <output from previous rows> + '<element_name>'
390+
*
391+
* If all the column values are nulls, this element should be
392+
* <element_name/>, modify the already appended <element_name> to
393+
* <element_name/>.
394+
*/
395+
if (state->len > inital_state_len) // sanity check, should always be true
396+
{
397+
state->data[state->len - 1] = '/';
398+
appendStringInfoString(state, ">");
399+
}
400+
}
401+
else
402+
appendStringInfo(state, "</%s>", element_name);
403+
}
391404
}
392405
ReleaseTupleDesc(tupdesc);
393406
}

test/JDBC/expected/forxml-path-vu-cleanup.out

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,8 @@ DROP VIEW for_xml_path_v3
1010
DROP VIEW for_xml_path_v4
1111
GO
1212

13+
DROP TABLE for_xml_path_all_null
14+
GO
15+
1316
DROP TABLE for_xml_path
1417
GO

test/JDBC/expected/forxml-path-vu-prepare.out

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,20 @@ GO
66
~~ROW COUNT: 9~~
77

88

9+
CREATE TABLE for_xml_path_all_null (col1 INT NULL);
10+
GO
11+
12+
SET NOCOUNT ON
13+
GO
14+
15+
DECLARE @i INT = 1;
16+
WHILE @i <= 2048
17+
BEGIN
18+
INSERT INTO for_xml_path_all_null VALUES (NULL);
19+
SET @i = @i + 1;
20+
END
21+
GO
22+
923
CREATE VIEW for_xml_path_v1 AS SELECT String as param1, hello.String as [@param2] FROM for_xml_path hello ORDER BY SequenceNumber FOR XML PATH('hello')
1024
GO
1125

test/JDBC/expected/forxml-path-vu-verify.out

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,3 +123,29 @@ ntext
123123
SELECT Product, UnitPrice, EffectiveDate FROM Products WHERE UnitPrice &gt; 100
124124
~~END~~
125125

126+
127+
-- BABEL-5364
128+
-- Crash causing query
129+
SELECT col1 from for_xml_path_all_null FOR XML PATH('')
130+
GO
131+
~~START~~
132+
ntext
133+
~~END~~
134+
135+
136+
SELECT col1 from for_xml_path_all_null FOR XML PATH(''), TYPE
137+
GO
138+
~~START~~
139+
xml
140+
141+
~~END~~
142+
143+
144+
SELECT Top 10 col1 from for_xml_path_all_null FOR XML PATH('element')
145+
GO
146+
~~START~~
147+
ntext
148+
<element/><element/><element/><element/><element/><element/><element/><element/><element/><element/>
149+
~~END~~
150+
151+

test/JDBC/input/forxml/forxml-path-vu-cleanup.sql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,8 @@ DROP VIEW for_xml_path_v3
1010
DROP VIEW for_xml_path_v4
1111
GO
1212

13+
DROP TABLE for_xml_path_all_null
14+
GO
15+
1316
DROP TABLE for_xml_path
1417
GO

test/JDBC/input/forxml/forxml-path-vu-prepare.sql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,20 @@ GO
44
INSERT INTO for_xml_path VALUES (1,'SELECT'),(2,'Product,'),(3,'UnitPrice,'),(4,'EffectiveDate'),(5,'FROM'), (6,'Products'),(7,'WHERE'),(8,'UnitPrice'),(9,'> 100');
55
GO
66

7+
CREATE TABLE for_xml_path_all_null (col1 INT NULL);
8+
GO
9+
10+
SET NOCOUNT ON
11+
GO
12+
13+
DECLARE @i INT = 1;
14+
WHILE @i <= 2048
15+
BEGIN
16+
INSERT INTO for_xml_path_all_null VALUES (NULL);
17+
SET @i = @i + 1;
18+
END
19+
GO
20+
721
CREATE VIEW for_xml_path_v1 AS SELECT String as param1, hello.String as [@param2] FROM for_xml_path hello ORDER BY SequenceNumber FOR XML PATH('hello')
822
GO
923

test/JDBC/input/forxml/forxml-path-vu-verify.sql

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,15 @@ GO
4747

4848
EXEC for_xml_path_p4
4949
GO
50+
51+
-- BABEL-5364
52+
-- Crash causing query
53+
SELECT col1 from for_xml_path_all_null FOR XML PATH('')
54+
GO
55+
56+
SELECT col1 from for_xml_path_all_null FOR XML PATH(''), TYPE
57+
GO
58+
59+
SELECT Top 10 col1 from for_xml_path_all_null FOR XML PATH('element')
60+
GO
61+

0 commit comments

Comments
 (0)