Copilot commented on code in PR #13708:
URL: https://github.com/apache/trafficserver/pull/13708#discussion_r4236660382
##########
plugins/experimental/access_control/access_control.cc:
##########
@@ -23,12 +23,123 @@
#include <iostream>
#include <string>
+#include <charconv>
#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, bool isScope = false)
+{
+ normalized.clear();
+ if (path.empty()) {
+ normalized = "/";
+ return true;
+ }
+
+ normalized.reserve(path.size() + 1);
+ std::vector<size_t> segment_starts;
+ 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 == ".") {
+ normalized.resize(seg_start);
+ } else if (seg == "..") {
+ if (isScope) {
+ return false;
+ }
+
+ normalized.resize(seg_start);
+
+ if (!segment_starts.empty()) {
+ normalized.resize(segment_starts.back());
+ segment_starts.pop_back();
+ }
Review Comment:
This resolves `.`/`..` segments only for the authorization comparison, but
ATS preserves those segments in the URL path (the URL parser only strips
leading slashes), so the origin can receive `/reports/../hr` while a token
scoped to `/hr` is accepted. That makes the authorization decision depend on
whether the origin independently canonicalizes the path; reject dot segments
(including encoded dot segments) or canonicalize the actual forwarded URL
before matching to avoid a path-confusion bypass.
--
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]
