From ce241fa40adee2b12f8e225db98e09a45bc2acbb Mon Sep 17 00:00:00 2001
From: Crystal <crystal@noreply.codeberg.org>
Date: Mon, 20 Mar 2023 22:57:26 +0000
Subject: [PATCH 1/4] Fix certificate renewal (#209)
A database bug in xorm.go prevents the pages-server from saving a
renewed certificate for a domain that already has one in the database.
Co-authored-by: crystal <crystal@noreply.codeberg.org>
Co-authored-by: 6543 <6543@obermui.de>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/209
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: Crystal <crystal@noreply.codeberg.org>
Co-committed-by: Crystal <crystal@noreply.codeberg.org>
---
server/database/xorm.go | 2 +-
server/database/xorm_test.go | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/server/database/xorm.go b/server/database/xorm.go
index a14f887..32d5bc2 100644
--- a/server/database/xorm.go
+++ b/server/database/xorm.go
@@ -64,7 +64,7 @@ func (x xDB) Put(domain string, cert *certificate.Resource) error {
}
defer sess.Close()
- if exist, _ := sess.ID(c.Domain).Exist(); exist {
+ if exist, _ := sess.ID(c.Domain).Exist(new(Cert)); exist {
if _, err := sess.ID(c.Domain).Update(c); err != nil {
return err
}
diff --git a/server/database/xorm_test.go b/server/database/xorm_test.go
index 9c032ee..50d8a7f 100644
--- a/server/database/xorm_test.go
+++ b/server/database/xorm_test.go
@@ -37,7 +37,7 @@ func TestSanitizeWildcardCerts(t *testing.T) {
}))
// update existing cert
- assert.Error(t, certDB.Put(".wildcard.de", &certificate.Resource{
+ assert.NoError(t, certDB.Put(".wildcard.de", &certificate.Resource{
Domain: "*.wildcard.de",
Certificate: localhost_mock_directory_certificate,
}))
From b7bf7458630e7bf01a4ac63816b03eeae5c21b64 Mon Sep 17 00:00:00 2001
From: Moritz Marquardt <git@momar.de>
Date: Sun, 27 Aug 2023 10:13:15 +0200
Subject: [PATCH 2/4] Security Fix: clean paths correctly to avoid
circumvention of BlacklistedPaths
---
server/context/context.go | 6 ++--
server/utils/utils.go | 14 ++++++++++
server/utils/utils_test.go | 56 ++++++++++++++++++++++++++++++++++++++
3 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/server/context/context.go b/server/context/context.go
index 481fee2..6650164 100644
--- a/server/context/context.go
+++ b/server/context/context.go
@@ -48,11 +48,9 @@ func (c *Context) Redirect(uri string, statusCode int) {
http.Redirect(c.RespWriter, c.Req, uri, statusCode)
}
-// Path returns requested path.
-//
-// The returned bytes are valid until your request handler returns.
+// Path returns the cleaned requested path.
func (c *Context) Path() string {
- return c.Req.URL.Path
+ return utils.CleanPath(c.Req.URL.Path)
}
func (c *Context) Host() string {
diff --git a/server/utils/utils.go b/server/utils/utils.go
index 30f948d..91ed359 100644
--- a/server/utils/utils.go
+++ b/server/utils/utils.go
@@ -1,6 +1,8 @@
package utils
import (
+ "net/url"
+ "path"
"strings"
)
@@ -11,3 +13,15 @@ func TrimHostPort(host string) string {
}
return host
}
+
+func CleanPath(uriPath string) string {
+ unescapedPath, _ := url.PathUnescape(uriPath)
+ cleanedPath := path.Join("/", unescapedPath)
+
+ // If the path refers to a directory, add a trailing slash.
+ if !strings.HasSuffix(cleanedPath, "/") && (strings.HasSuffix(unescapedPath, "/") || strings.HasSuffix(unescapedPath, "/.") || strings.HasSuffix(unescapedPath, "/..")) {
+ cleanedPath += "/"
+ }
+
+ return cleanedPath
+}
diff --git a/server/utils/utils_test.go b/server/utils/utils_test.go
index 2532392..b8fcea9 100644
--- a/server/utils/utils_test.go
+++ b/server/utils/utils_test.go
@@ -11,3 +11,59 @@ func TestTrimHostPort(t *testing.T) {
assert.EqualValues(t, "", TrimHostPort(":"))
assert.EqualValues(t, "example.com", TrimHostPort("example.com:80"))
}
+
+// TestCleanPath is mostly copied from fasthttp, to keep the behaviour we had before migrating away from it.
+// Source (MIT licensed): https://github.com/valyala/fasthttp/blob/v1.48.0/uri_test.go#L154
+// Copyright (c) 2015-present Aliaksandr Valialkin, VertaMedia, Kirill Danshin, Erik Dubbelboer, FastHTTP Authors
+func TestCleanPath(t *testing.T) {
+ // double slash
+ testURIPathNormalize(t, "/aa//bb", "/aa/bb")
+
+ // triple slash
+ testURIPathNormalize(t, "/x///y/", "/x/y/")
+
+ // multi slashes
+ testURIPathNormalize(t, "/abc//de///fg////", "/abc/de/fg/")
+
+ // encoded slashes
+ testURIPathNormalize(t, "/xxxx%2fyyy%2f%2F%2F", "/xxxx/yyy/")
+
+ // dotdot
+ testURIPathNormalize(t, "/aaa/..", "/")
+
+ // dotdot with trailing slash
+ testURIPathNormalize(t, "/xxx/yyy/../", "/xxx/")
+
+ // multi dotdots
+ testURIPathNormalize(t, "/aaa/bbb/ccc/../../ddd", "/aaa/ddd")
+
+ // dotdots separated by other data
+ testURIPathNormalize(t, "/a/b/../c/d/../e/..", "/a/c/")
+
+ // too many dotdots
+ testURIPathNormalize(t, "/aaa/../../../../xxx", "/xxx")
+ testURIPathNormalize(t, "/../../../../../..", "/")
+ testURIPathNormalize(t, "/../../../../../../", "/")
+
+ // encoded dotdots
+ testURIPathNormalize(t, "/aaa%2Fbbb%2F%2E.%2Fxxx", "/aaa/xxx")
+
+ // double slash with dotdots
+ testURIPathNormalize(t, "/aaa////..//b", "/b")
+
+ // fake dotdot
+ testURIPathNormalize(t, "/aaa/..bbb/ccc/..", "/aaa/..bbb/")
+
+ // single dot
+ testURIPathNormalize(t, "/a/./b/././c/./d.html", "/a/b/c/d.html")
+ testURIPathNormalize(t, "./foo/", "/foo/")
+ testURIPathNormalize(t, "./../.././../../aaa/bbb/../../../././../", "/")
+ testURIPathNormalize(t, "./a/./.././../b/./foo.html", "/b/foo.html")
+}
+
+func testURIPathNormalize(t *testing.T, requestURI, expectedPath string) {
+ cleanedPath := CleanPath(requestURI)
+ if cleanedPath != expectedPath {
+ t.Fatalf("Unexpected path %q. Expected %q. requestURI=%q", cleanedPath, expectedPath, requestURI)
+ }
+}
From 5a6f4154280b0ab6354265b83e71954b8f19e728 Mon Sep 17 00:00:00 2001
From: Moritz Marquardt <git@momar.de>
Date: Sun, 27 Aug 2023 11:10:55 +0200
Subject: [PATCH 3/4] Fix CI pipeline (replace "pipeline" with "steps")
---
.woodpecker.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.woodpecker.yml b/.woodpecker.yml
index ba44f82..75c82f3 100644
--- a/.woodpecker.yml
+++ b/.woodpecker.yml
@@ -1,4 +1,4 @@
-pipeline:
+steps:
# use vendor to cache dependencies
vendor:
image: golang:1.20
From 5fe4613813898374834e20842bfb80088835db15 Mon Sep 17 00:00:00 2001
From: 6543 <6543@obermui.de>
Date: Mon, 17 Jul 2023 19:44:58 +0000
Subject: [PATCH 4/4] Use http.NoBody as per linter (#231)
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/231
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
---
server/handler/handler_test.go | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/server/handler/handler_test.go b/server/handler/handler_test.go
index 626564a..d716ef4 100644
--- a/server/handler/handler_test.go
+++ b/server/handler/handler_test.go
@@ -1,6 +1,7 @@
package handler
import (
+ "net/http"
"net/http/httptest"
"testing"
"time"
@@ -24,7 +25,7 @@ func TestHandlerPerformance(t *testing.T) {
testCase := func(uri string, status int) {
t.Run(uri, func(t *testing.T) {
- req := httptest.NewRequest("GET", uri, nil)
+ req := httptest.NewRequest("GET", uri, http.NoBody)
w := httptest.NewRecorder()
log.Printf("Start: %v\n", time.Now())