You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "ubeddulla (via GitHub)" <gi...@apache.org> on 2026/10/08 01:15:40 UTC
[PR] validate query parameters echoed into builtin html pages (brpc)
ubeddulla opened a new pull request, #3589:
URL: https://github.com/apache/brpc/pull/3589
### What problem does this PR solve?
Issue Number: resolve N/A
Problem Summary:
1. `StartProfiling` writes the `view` and `base` query parameters straight into the inline `<script>` of the `/hotspots/{cpu,heap,growth,contention,iobuf}` page, as part of the url of the `$.ajax` call it makes to `*_non_responsive`. `GET /hotspots/cpu?view=" + alert(1) + "</script><script>alert(2)</script>` therefore closes the js string and the script element and runs attacker-chosen script in the origin that also serves `/flags?setvalue=` and the rest of the builtin admin endpoints.
2. `DoProfiling`, the handler that same ajax call lands on, already rejects both parameters with `ValidProfilePath()`, and the comment on that helper says it has to be applied to every parameter that comes from the url. The page handler just never calls it, so applying it there too closes the injection and makes the two handlers agree on what they accept.
3. `/rpcz` is the only other builtin page that echoes a query parameter, and it prints `time_str` (a `const std::string*`) rather than `*time_str`, so an unparsable value renders a heap address instead: ``Invalid time=`0x10a9e9168'``.
### What is changed and the side effects?
Changed:
`ValidProfilePath()` on `view` and `base` in `StartProfiling`, failing with the same `EINVAL` text the sibling handler uses, and `WebEscape()` on the html branch of the `/rpcz` message. Profile paths the page itself generates (`./rpc_data/profiling/<checksum>/<timestamp>.<type>`) pass the check, so links from the page keep working.
Side effects:
- Performance effects: none, one validation of two short query values per `/hotspots/*` page load.
- Breaking backward compatibility: none. A `view`/`base` that the check rejects is one `*_non_responsive` already refused, it now fails on the page instead of in the ajax response. Plain text `/rpcz` output is unchanged.
---
### Check List:
- Added `BuiltinServiceTest.hotspots_query_escaping` and `BuiltinServiceTest.rpcz_query_escaping` in `test/brpc_builtin_service_unittest.cpp`. Both fail before the fix: the first shows `url: "/hotspots/cpu_non_responsive?console=1&seconds=10&display_type=dot&view=" + alert(1) + "</script><script>alert(2)</script>",` in the rendered page, the second shows the leaked pointer. Both pass after.
- `brpc_builtin_service_unittest` is green (21/22) via cmake with `-DBUILD_UNIT_TESTS=ON`. The one failure is `BuiltinServiceTest.dir`, which expects `/usr/include/sys/errno.h` and fails on this macOS box before the change as well.
- Compiles clean with `-Wall -W` in both the debug and the release configuration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] validate query parameters echoed into builtin html pages (brpc)
Posted by "wasphin (via GitHub)" <gi...@apache.org>.
wasphin merged PR #3589:
URL: https://github.com/apache/brpc/pull/3589
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] validate query parameters echoed into builtin html pages (brpc)
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3589:
URL: https://github.com/apache/brpc/pull/3589#discussion_r4214009112
##########
src/brpc/builtin/rpcz_service.cpp:
##########
@@ -606,7 +606,8 @@ void RpczService::default_method(::google::protobuf::RpcController* cntl_base,
} else {
start_tm = ParseDateTime(*time_str);
if (start_tm < 0) {
- os << "Invalid " << TIME_STR << "=`" << time_str << '\''
+ os << "Invalid " << TIME_STR << "=`"
+ << (use_html ? WebEscape(*time_str) : *time_str) << '\''
Review Comment:
The PR description says plain-text `/rpcz` output is unchanged, but this branch changes it from streaming the `std::string*` address to streaming the actual invalid query value (`*time_str`). That behavior change appears intentional to fix the pointer leak, so please update the stated side effects (and, ideally, add a plain-text assertion to make the intended behavior explicit).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org