cmcfarlen commented on code in PR #13708:
URL: https://github.com/apache/trafficserver/pull/13708#discussion_r4273808620
##########
plugins/experimental/access_control/access_control.cc:
##########
@@ -23,12 +23,105 @@
#include <iostream>
#include <string>
+#include <charconv>
+#include <system_error>
#include "access_control.h"
size_t calcMessageDigest(const StringView hf, const char *secret, const char
*message, size_t messageLen, char *buffer, size_t len);
const char *getSecretMap(const StringMap &map, const StringView &key, size_t
&secretSize);
+static bool
+normalizePath(StringView path, String &normalized)
+{
+ normalized.clear();
+ if (path.empty()) {
+ normalized = "/";
+ return true;
+ }
+
+ normalized.reserve(path.size() + 1);
+ size_t i = 0;
+
+ while (i < path.size()) {
+ while (i < path.size() && path[i] == '/') {
+ ++i;
+ }
+ if (i >= path.size()) {
+ break;
+ }
+
+ size_t seg_start = normalized.size();
+ normalized.push_back('/');
+
+ while (i < path.size() && path[i] != '/') {
+ if (path[i] == '\\') {
+ return false;
+ }
+ if (path[i] == '%') {
+ unsigned int val = 0;
+ if (i + 2 >= path.size()) {
+ return false;
+ }
+ auto [end, ec] = std::from_chars(path.data() + i + 1, path.data() + i
+ 3, val, 16);
+ if (end == path.data() + i + 3 && ec == std::errc{} && val <= 0xFF) {
+ if (val == '/' || val == '\\' || val < 0x20 || val == 0x7F) {
+ return false;
+ }
+ normalized.push_back(static_cast<char>(val));
+ i += 3;
+ } else {
+ return false;
+ }
+ } else {
+ normalized.push_back(path[i]);
+ ++i;
+ }
+ }
+
+ StringView seg(normalized.data() + seg_start + 1, normalized.size() -
seg_start - 1);
+
+ if (seg == "." || seg == "..") {
Review Comment:
Thanks for switching to rejection — plain and encoded `..` are now refused.
Two forms from my earlier comment still get through, though. I ran
`validateScopeDetailed()` against a `/reports` scope:
```
reports/..;/hr/payroll -> IN_SCOPE
reports/.;/x -> IN_SCOPE
reports/%2e%2e%3b/hr -> IN_SCOPE
reports/%252e%252e/hr -> IN_SCOPE
```
ATS 10 keeps `;params` inside the path (`URL.cc`, "That parsing was removed,
so ";params" now stays inside the path"), so the plugin sees `..;` verbatim,
and Tomcat/Spring resolve it as `..`. And because `%25` decodes to a kept `%`,
double-encoded dots survive the single decode for an origin that decodes again.
Comparing only the part of the segment before the first `;` against `.`/`..`,
and rejecting a decoded `%`, would close both. Unit cases for these four would
pin it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
