mirror of
https://github.com/9001/copyparty.git
synced 2025-08-18 09:22:31 -06:00
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.
This commit is contained in:
parent
335fcc8535
commit
007d948cb9
|
@ -337,6 +337,15 @@ class HttpCli(object):
|
||||||
vpath, arglist = self.req.split("?", 1)
|
vpath, arglist = self.req.split("?", 1)
|
||||||
self.trailing_slash = vpath.endswith("/")
|
self.trailing_slash = vpath.endswith("/")
|
||||||
vpath = undot(vpath)
|
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("&"):
|
for k in arglist.split("&"):
|
||||||
if "=" in k:
|
if "=" in k:
|
||||||
k, zs = k.split("=", 1)
|
k, zs = k.split("=", 1)
|
||||||
|
@ -488,6 +497,9 @@ class HttpCli(object):
|
||||||
pex: Pebkac = ex # type: ignore
|
pex: Pebkac = ex # type: ignore
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
if pex.code == 999:
|
||||||
|
return False
|
||||||
|
|
||||||
post = self.mode in ["POST", "PUT"] or "content-length" in self.headers
|
post = self.mode in ["POST", "PUT"] or "content-length" in self.headers
|
||||||
if not self._check_nonfatal(pex, post):
|
if not self._check_nonfatal(pex, post):
|
||||||
self.keepalive = False
|
self.keepalive = False
|
||||||
|
@ -586,6 +598,14 @@ class HttpCli(object):
|
||||||
for k, zs in list(self.out_headers.items()) + self.out_headerlist:
|
for k, zs in list(self.out_headers.items()) + self.out_headerlist:
|
||||||
response.append("%s: %s" % (k, zs))
|
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:
|
try:
|
||||||
# best practice to separate headers and body into different packets
|
# 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")
|
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")
|
path_base = os.path.join(self.E.mod, "web")
|
||||||
static_path = absreal(os.path.join(path_base, self.vpath[5:]))
|
static_path = absreal(os.path.join(path_base, self.vpath[5:]))
|
||||||
if not static_path.startswith(path_base):
|
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.log(t.format(self.vpath, static_path), 1)
|
||||||
self.tx_404()
|
self.tx_404()
|
||||||
return False
|
return False
|
||||||
|
@ -3077,7 +3097,14 @@ class HttpCli(object):
|
||||||
return True
|
return True
|
||||||
|
|
||||||
def set_k304(self) -> bool:
|
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.out_headerlist.append(("Set-Cookie", ck))
|
||||||
self.redirect("", "?h#cc")
|
self.redirect("", "?h#cc")
|
||||||
return True
|
return True
|
||||||
|
|
|
@ -4,6 +4,7 @@ from __future__ import print_function, unicode_literals
|
||||||
import base64
|
import base64
|
||||||
import math
|
import math
|
||||||
import os
|
import os
|
||||||
|
import re
|
||||||
import socket
|
import socket
|
||||||
import sys
|
import sys
|
||||||
import threading
|
import threading
|
||||||
|
@ -138,6 +139,8 @@ class HttpSrv(object):
|
||||||
zs = os.path.join(self.E.mod, "web", "deps", "prism.js.gz")
|
zs = os.path.join(self.E.mod, "web", "deps", "prism.js.gz")
|
||||||
self.prism = os.path.exists(zs)
|
self.prism = os.path.exists(zs)
|
||||||
|
|
||||||
|
self.ptn_cc = re.compile(r"[\x00-\x1f]")
|
||||||
|
|
||||||
self.mallow = "GET HEAD POST PUT DELETE OPTIONS".split()
|
self.mallow = "GET HEAD POST PUT DELETE OPTIONS".split()
|
||||||
if not self.args.no_dav:
|
if not self.args.no_dav:
|
||||||
zs = "PROPFIND PROPPATCH LOCK UNLOCK MKCOL COPY MOVE"
|
zs = "PROPFIND PROPPATCH LOCK UNLOCK MKCOL COPY MOVE"
|
||||||
|
|
|
@ -171,6 +171,7 @@ HTTPCODE = {
|
||||||
500: "Internal Server Error",
|
500: "Internal Server Error",
|
||||||
501: "Not Implemented",
|
501: "Not Implemented",
|
||||||
503: "Service Unavailable",
|
503: "Service Unavailable",
|
||||||
|
999: "MissingNo",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue