Skip to content

[fix](statistics) Quote identifiers in statistics queries - #67703

Open
morrySnow wants to merge 2 commits into
apache:masterfrom
morrySnow:fix/quote-statistics-identifiers
Open

[fix](statistics) Quote identifiers in statistics queries#67703
morrySnow wants to merge 2 commits into
apache:masterfrom
morrySnow:fix/quote-statistics-identifiers

Conversation

@morrySnow

@morrySnow morrySnow commented Sep 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Problem

Statistics collection builds internal SQL from catalog, database, table, index, partition, and column names. Names containing an embedded backtick could terminate a template-owned quoted identifier. Since statistics statements execute through an internal privileged context, the remaining name text could be interpreted as SQL and access objects unavailable to the initiating user.

Root cause

The SQL templates owned the surrounding backticks while callers substituted raw names, or only partially escaped selected column names. This split ownership was inconsistent across full, sampled, partition, external-table, plugin-driven, and histogram collection paths. It also made it easy for a newly added template expression to consume an unescaped name.

How to reproduce

  1. Create a low-privilege user that can create and analyze tables in one database but cannot read a table in another database.
  2. In the permitted database, create a table whose quoted name contains an embedded backtick followed by a crafted SQL fragment such as a UNION query against the inaccessible table.
  3. Trigger synchronous statistics collection for that table.
  4. Before this change, the embedded backtick closes the template's identifier and the remaining text is parsed in the privileged statistics context. Values from the inaccessible table can consequently appear in the attacker's statistics rows.

Fix

  • Build every object-name fragment with SqlUtils.getIdentSql, so embedded backticks are doubled and the complete value remains one identifier.
  • Make templates consume already quoted identifier fragments instead of adding their own backticks.
  • Apply the same model to OLAP, external-table, plugin-driven sample, partition, index, and histogram SQL generation.
  • Keep metadata values used as string literals on a separate SQL-literal escaping path.
  • Add unit coverage for catalog, database, table, index, partition, and column names containing embedded backticks, including a column name that also contains an apostrophe.

Tests

  • OlapAnalysisTaskTest: 25 tests passed.
  • HistogramTaskTest: 2 tests passed.
  • AnalyzeTest: 2 tests passed.
  • BaseAnalysisTaskTest: 3 tests passed.
  • Re-triggered FE UT build: passed.

@morrySnow
morrySnow requested a review from englefly as a code owner September 9, 2026 04:32
@hello-stephen

hello-stephen commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@morrySnow

morrySnow commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@morrySnow morrySnow changed the title [fix](fe) Quote identifiers in statistics queries [fix](statistics) Quote identifiers in statistics queries Sep 9, 2026
@hello-stephen

hello-stephen commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 16.52% (19/115) 🎉
Increment coverage report
Complete coverage report

@hello-stephen

hello-stephen commented Sep 9, 2026

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 17144 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 941a33dd991978a1f66fe6080f124eb717fb252d, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17570	3038	3013	3013
q2	2122	267	234	234
q3	10198	882	522	522
q4	4674	251	209	209
q5	7666	540	387	387
q6	131	116	94	94
q7	533	501	391	391
q8	9238	923	955	923
q9	3484	2420	2418	2418
q10	6518	908	735	735
q11	407	198	184	184
q12	618	273	201	201
q13	18118	1548	1174	1174
q14	158	148	138	138
q15	q16	443	401	368	368
q17	1356	838	835	835
q18	3154	2330	2333	2330
q19	1254	908	764	764
q20	394	295	213	213
q21	5617	1783	1861	1783
q22	328	274	228	228
Total cold run time: 93981 ms
Total hot run time: 17144 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3364	3328	3310	3310
q2	528	406	385	385
q3	2307	2371	2176	2176
q4	1219	1198	889	889
q5	2279	2168	2188	2168
q6	169	118	88	88
q7	1046	931	919	919
q8	1595	1378	1386	1378
q9	3233	3216	3186	3186
q10	1907	1861	1679	1679
q11	357	279	254	254
q12	452	436	343	343
q13	1485	1544	1182	1182
q14	174	171	163	163
q15	q16	391	401	356	356
q17	3636	3392	3395	3392
q18	4946	4571	5102	4571
q19	942	861	841	841
q20	1020	1007	848	848
q21	3838	3182	3204	3182
q22	393	353	336	336
Total cold run time: 35281 ms
Total hot run time: 31646 ms

@hello-stephen

hello-stephen commented Sep 9, 2026

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 82534 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 941a33dd991978a1f66fe6080f124eb717fb252d, data reload: false

query5	4232	420	341	341
query6	384	138	121	121
query7	4945	415	229	229
query8	286	126	118	118
query9	8692	2874	2872	2872
query10	403	217	181	181
query11	5378	1060	927	927
query12	119	72	71	71
query13	1195	427	316	316
query14	6062	2232	2140	2140
query14_1	2033	2015	1999	1999
query15	175	123	114	114
query16	932	373	341	341
query17	788	455	368	368
query18	2337	308	269	269
query19	167	134	110	110
query20	74	70	72	70
query21	197	99	85	85
query22	5479	5485	5449	5449
query23	6888	6365	6065	6065
query23_1	6165	6162	6293	6162
query24	7273	1119	766	766
query24_1	775	768	798	768
query25	422	320	262	262
query26	1227	219	135	135
query27	2789	397	246	246
query28	4717	1500	1491	1491
query29	934	459	325	325
query30	247	159	130	130
query31	809	395	337	337
query32	120	71	78	71
query33	448	204	173	173
query34	968	807	492	492
query35	405	402	339	339
query36	560	568	558	558
query37	123	78	72	72
query38	1008	855	824	824
query39	497	475	457	457
query39_1	461	449	473	449
query40	200	91	77	77
query41	55	52	50	50
query42	73	74	73	73
query43	243	242	212	212
query44	1013	545	525	525
query45	109	107	99	99
query46	755	822	529	529
query47	766	755	721	721
query48	308	306	213	213
query49	560	232	199	199
query50	749	255	189	189
query51	8016	8003	7943	7943
query52	66	75	60	60
query53	202	199	144	144
query54	218	158	153	153
query55	81	58	53	53
query56	179	166	158	158
query57	767	667	679	667
query58	200	174	164	164
query59	1274	1245	1130	1130
query60	237	186	179	179
query61	111	111	111	111
query62	339	197	183	183
query63	172	142	140	140
query64	2670	674	584	584
query65	1711	1603	1675	1603
query66	1879	316	226	226
query67	10219	9963	9736	9736
query68	2782	1147	696	696
query69	344	245	207	207
query70	670	649	620	620
query71	251	213	161	161
query72	2250	1653	1674	1653
query73	621	584	354	354
query74	1569	1259	1150	1150
query75	1188	1112	981	981
query76	2284	718	513	513
query77	251	260	209	209
query78	4119	3783	3287	3287
query79	1990	819	582	582
query80	1249	330	281	281
query81	489	155	130	130
query82	623	131	94	94
query83	266	208	189	189
query84	285	109	87	87
query85	762	341	278	278
query86	384	181	169	169
query87	1043	974	915	915
query88	2751	2095	2095	2095
query89	291	194	175	175
query90	1982	124	130	124
query91	127	124	96	96
query92	81	59	68	59
query93	1346	1057	680	680
query94	623	254	225	225
query95	519	338	223	223
query96	863	548	262	262
query97	1097	1105	1047	1047
query98	140	137	134	134
query99	418	349	307	307
Total cold run time: 176960 ms
Total hot run time: 82534 ms

@hello-stephen

hello-stephen commented Sep 9, 2026

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 14.72 s 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 941a33dd991978a1f66fe6080f124eb717fb252d, data reload: false

query1	0.01	0.01	0.00
query2	0.08	0.04	0.03
query3	0.25	0.10	0.09
query4	1.60	0.11	0.09
query5	0.17	0.16	0.15
query6	1.25	0.69	0.70
query7	0.04	0.01	0.00
query8	0.05	0.03	0.04
query9	0.28	0.21	0.21
query10	0.34	0.37	0.34
query11	0.16	0.11	0.12
query12	0.15	0.12	0.11
query13	0.30	0.30	0.30
query14	0.45	0.48	0.46
query15	0.35	0.35	0.36
query16	0.22	0.23	0.24
query17	0.69	0.67	0.67
query18	0.16	0.17	0.16
query19	1.21	1.18	1.15
query20	0.02	0.01	0.01
query21	15.46	0.17	0.12
query22	5.08	0.04	0.04
query23	16.16	0.25	0.11
query24	2.99	0.32	0.25
query25	0.10	0.03	0.04
query26	0.77	0.16	0.13
query27	0.03	0.03	0.04
query28	3.64	0.54	0.26
query29	12.43	3.19	2.60
query30	0.26	0.11	0.11
query31	2.76	0.39	0.17
query32	3.51	0.33	0.23
query33	1.42	1.39	1.48
query34	15.37	2.20	1.80
query35	1.74	1.75	1.77
query36	0.46	0.29	0.29
query37	0.06	0.04	0.04
query38	0.05	0.03	0.03
query39	0.03	0.02	0.02
query40	0.12	0.09	0.07
query41	0.07	0.03	0.02
query42	0.03	0.03	0.02
query43	0.04	0.03	0.03
Total cold run time: 90.36 s
Total hot run time: 14.72 s

@morrySnow

morrySnow commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@morrySnow
[fix](fe) Quote identifiers in statistics queries
49a673d 
Problem Summary:
Statistics collection SQL placed object names inside template-owned backticks without escaping embedded backticks. Because these statements run with an elevated internal identity, a crafted object name could alter the generated query and expose data outside the caller's privileges.

Solution:
Generate complete quoted identifier fragments with SqlUtils.getIdentSql for catalogs, databases, tables, columns, indexes, and partitions across OLAP, external, plugin-sample, and histogram analysis paths. Escape identifier-derived values separately when stored as string literals, and add coverage for embedded backticks and apostrophes.

Issue Number: None
@morrySnow
[fix](statistics) Update tests for quoted identifiers
7376cde 
Issue Number: None

Keep the statistics task test fixtures and template expectations consistent with pre-quoted identifier substitutions.
@morrySnow
morrySnow force-pushed the fix/quote-statistics-identifiers branch from 2ebe95d to 7376cde Compare September 10, 2026 06:42
@morrySnow

morrySnow commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@englefly englefly Awaiting requested review from englefly englefly is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@morrySnow @hello-stephen