Skip to content

Commit 9c2265b

Browse files
committed
Fix crash in tsql_row_to_xml_path function (babelfish-for-postgresql#4661)
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 ee6aa72 commit 9c2265b

File tree

7 files changed

+109
-24
lines changed

7 files changed

+109
-24
lines changed

contrib/babelfishpg_tsql/src/tsql_for/forxml.c

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,10 @@ tsql_query_to_xml_text_ffunc(PG_FUNCTION_ARGS)
143143
{
144144
StringInfo res = for_xml_ffunc(fcinfo);
145145

146+
/* return NULL if empty result (i.e. no rows) */
147+
if (res->len == 0)
148+
PG_RETURN_NULL();
149+
146150
PG_RETURN_TEXT_P(cstring_to_text_with_len(res->data, res->len));
147151
}
148152

@@ -313,7 +317,7 @@ validate_attribute_centric_col_names_xml(const char *element_name, TupleDesc tup
313317
seen_non_att_centric = true;
314318
}
315319

316-
if(seen_att_centric && element_name[0] == '\0')
320+
if(seen_att_centric && (element_name && strlen(element_name) == 0))
317321
{
318322
ereport(ERROR,
319323
(errcode(ERRCODE_INVALID_XML_PROCESSING_INSTRUCTION),
@@ -331,14 +335,15 @@ static void
331335
tsql_row_to_xml_path(StringInfo state, Datum record, const char *element_name, bool binary_base64, bool xsinil)
332336
{
333337
HeapTupleHeader td;
334-
Oid tupType;
335-
int32 tupTypmod;
336-
TupleDesc tupdesc;
337-
HeapTupleData tmptup;
338-
HeapTuple tuple;
339-
bool allnull = true;
340-
bool has_att_centric = false;
341-
bool first = true;
338+
Oid tupType;
339+
int32 tupTypmod;
340+
TupleDesc tupdesc;
341+
HeapTupleData tmptup;
342+
HeapTuple tuple;
343+
bool allnull = true;
344+
bool has_att_centric = false;
345+
bool first = true;
346+
int inital_state_len = state->len;
342347

343348
td = DatumGetHeapTupleHeader(record);
344349

@@ -358,7 +363,7 @@ tsql_row_to_xml_path(StringInfo state, Datum record, const char *element_name, b
358363
* each tuple is either contained in a "row" tag, or standalone if the
359364
* element_name is an empty string
360365
*/
361-
if (element_name[0] != '\0')
366+
if (element_name && strlen(element_name) > 0)
362367
{
363368
/* if "''" is the input path, ignore it per TSQL behavior */
364369
if (has_att_centric)
@@ -420,7 +425,7 @@ tsql_row_to_xml_path(StringInfo state, Datum record, const char *element_name, b
420425
else
421426
{
422427
/* When PATH('') is used with XSINIL, add xmlns to each element */
423-
if (element_name[0] == '\0' && xsinil)
428+
if ((element_name && strlen(element_name) == 0) && xsinil)
424429
appendStringInfo(state, "<%s " XML_XMLNS_XSI ">%s</%s>",
425430
colname,
426431
map_sql_value_to_xml_value(colval, datatype_oid, true),
@@ -453,7 +458,7 @@ tsql_row_to_xml_path(StringInfo state, Datum record, const char *element_name, b
453458
if (strncmp(NameStr(att->attname), "?column?", 8) != 0)
454459
{
455460
/* When PATH('') is used with XSINIL, add xmlns to each element */
456-
if (element_name[0] == '\0')
461+
if (element_name && strlen(element_name) == 0)
457462
appendStringInfo(state, "<%s " XML_XMLNS_XSI " " XML_XSI_NIL "/>", colname);
458463
else
459464
appendStringInfo(state, "<%s " XML_XSI_NIL "/>", colname);
@@ -462,24 +467,32 @@ tsql_row_to_xml_path(StringInfo state, Datum record, const char *element_name, b
462467
}
463468
}
464469

465-
if (allnull)
466-
{
467-
/*
468-
* If all the column values are nulls, this element should be
469-
* <element_name/>, modify the already appended <element_name> to
470-
* <element_name/>.
471-
*/
472-
state->data[state->len - 1] = '/';
473-
appendStringInfoString(state, ">");
474-
}
475-
else if (element_name[0] != '\0')
470+
if (element_name && strlen(element_name) > 0)
476471
{
477472
if (has_att_centric && first)
478473
{
479474
appendStringInfoString(state, "/>");
480475
}
481476
else
482-
appendStringInfo(state, "</%s>", element_name);
477+
{
478+
if (allnull)
479+
{
480+
/*
481+
* At this point, state = <output from previous rows> + '<element_name>'
482+
*
483+
* If all the column values are nulls, this element should be
484+
* <element_name/>, modify the already appended <element_name> to
485+
* <element_name/>.
486+
*/
487+
if (state->len > inital_state_len) // sanity check, should always be true
488+
{
489+
state->data[state->len - 1] = '/';
490+
appendStringInfoString(state, ">");
491+
}
492+
}
493+
else
494+
appendStringInfo(state, "</%s>", element_name);
495+
}
483496
}
484497
ReleaseTupleDesc(tupdesc);
485498
}

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(''), ELEMENTS, 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(''), ELEMENTS, 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)