From 007d948cb982daa05bc6619cd20ee55b7e834c38 Mon Sep 17 00:00:00 2001 From: ed Date: Sun, 23 Jul 2023 10:55:08 +0000 Subject: [PATCH] fix GHSA-f54q-j679-p9hh: reflected-XSS in cookie-setters; it was possible to set cookie values which contained newlines, thus terminating the http header and bleeding into the body. We now disallow control-characters in queries, but still allow them in paths, as copyparty supports filenames containing newlines and other mojibake. The changes in `set_k304` are not necessary in fixing the vulnerability, but makes the behavior more correct. --- copyparty/httpcli.py | 31 +++++++++++++++++++++++++++++-- copyparty/httpsrv.py | 3 +++ copyparty/util.py | 1 + 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/copyparty/httpcli.py b/copyparty/httpcli.py index 97c04bbd..476c5ffb 100644 --- a/copyparty/httpcli.py +++ b/copyparty/httpcli.py @@ -337,6 +337,15 @@ class HttpCli(object): vpath, arglist = self.req.split("?", 1) self.trailing_slash = vpath.endswith("/") vpath = undot(vpath) + + zs = unquotep(arglist) + m = self.conn.hsrv.ptn_cc.search(zs) + if m: + hit = zs[m.span()[0] :] + t = "malicious user; Cc in query [{}] => [{!r}]" + self.log(t.format(self.req, hit), 1) + return False + for k in arglist.split("&"): if "=" in k: k, zs = k.split("=", 1) @@ -488,6 +497,9 @@ class HttpCli(object): pex: Pebkac = ex # type: ignore try: + if pex.code == 999: + return False + post = self.mode in ["POST", "PUT"] or "content-length" in self.headers if not self._check_nonfatal(pex, post): self.keepalive = False @@ -586,6 +598,14 @@ class HttpCli(object): for k, zs in list(self.out_headers.items()) + self.out_headerlist: response.append("%s: %s" % (k, zs)) + for zs in response: + m = self.conn.hsrv.ptn_cc.search(zs) + if m: + hit = zs[m.span()[0] :] + t = "malicious user; Cc in out-hdr {!r} => [{!r}]" + self.log(t.format(zs, hit), 1) + raise Pebkac(999) + try: # best practice to separate headers and body into different packets self.s.sendall("\r\n".join(response).encode("utf-8") + b"\r\n\r\n") @@ -785,7 +805,7 @@ class HttpCli(object): path_base = os.path.join(self.E.mod, "web") static_path = absreal(os.path.join(path_base, self.vpath[5:])) if not static_path.startswith(path_base): - t = "attempted path traversal [{}] => [{}]" + t = "malicious user; attempted path traversal [{}] => [{}]" self.log(t.format(self.vpath, static_path), 1) self.tx_404() return False @@ -3077,7 +3097,14 @@ class HttpCli(object): return True def set_k304(self) -> bool: - ck = gencookie("k304", self.uparam["k304"], self.args.R, False, 86400 * 299) + v = self.uparam["k304"].lower() + if v == "y": + dur = 86400 * 299 + else: + dur = None + v = "x" + + ck = gencookie("k304", v, self.args.R, False, dur) self.out_headerlist.append(("Set-Cookie", ck)) self.redirect("", "?h#cc") return True diff --git a/copyparty/httpsrv.py b/copyparty/httpsrv.py index f980a690..a9f55bc1 100644 --- a/copyparty/httpsrv.py +++ b/copyparty/httpsrv.py @@ -4,6 +4,7 @@ from __future__ import print_function, unicode_literals import base64 import math import os +import re import socket import sys import threading @@ -138,6 +139,8 @@ class HttpSrv(object): zs = os.path.join(self.E.mod, "web", "deps", "prism.js.gz") self.prism = os.path.exists(zs) + self.ptn_cc = re.compile(r"[\x00-\x1f]") + self.mallow = "GET HEAD POST PUT DELETE OPTIONS".split() if not self.args.no_dav: zs = "PROPFIND PROPPATCH LOCK UNLOCK MKCOL COPY MOVE" diff --git a/copyparty/util.py b/copyparty/util.py index d3eb06d1..fab74b79 100644 --- a/copyparty/util.py +++ b/copyparty/util.py @@ -171,6 +171,7 @@ HTTPCODE = { 500: "Internal Server Error", 501: "Not Implemented", 503: "Service Unavailable", + 999: "MissingNo", }