escape redirect urls (#783)

* escape additional redirect urls * excape query string in redirect * re-escape urls to avoid mixed-case hex from input url, in RawPath
ipfs · Jan 8, 2025 · ae7ab1f · ae7ab1f
1 parent 49d0e40
commit ae7ab1f
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 10 deletions.
diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go
@@ -700,13 +700,13 @@ func TestRedirects(t *testing.T) {
 			// - Browsers will send original URI in URL-escaped form
 			// - We expect query parameters to be persisted
 			// - We drop fragments, as those should not be sent by a browser
-			{"/ipfs/?uri=ipfs%3A%2F%2FQmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html%23header-%C4%85", http.StatusMovedPermanently, "/ipfs/QmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco/wiki/Foo_%c4%85%c4%99.html?filename=test-%c4%99.html"},
-			{"/ipfs/?uri=ipns%3A%2F%2Fexample.com%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html", http.StatusMovedPermanently, "/ipns/example.com/wiki/Foo_%c4%85%c4%99.html?filename=test-%c4%99.html"},
+			{"/ipfs/?uri=ipfs%3A%2F%2FQmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html%23header-%C4%85", http.StatusMovedPermanently, "/ipfs/QmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco/wiki/Foo_%C4%85%C4%99.html?filename=test-%C4%99.html"},
+			{"/ipfs/?uri=ipns%3A%2F%2Fexample.com%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html", http.StatusMovedPermanently, "/ipns/example.com/wiki/Foo_%C4%85%C4%99.html?filename=test-%C4%99.html"},
 			{"/ipfs/?uri=ipfs://" + cid, http.StatusMovedPermanently, "/ipfs/" + cid},
 			{"/ipfs?uri=ipfs://" + cid, http.StatusMovedPermanently, "/ipfs/" + cid},
 			{"/ipfs/?uri=ipns://" + cid, http.StatusMovedPermanently, "/ipns/" + cid},
-			{"/ipns/?uri=ipfs%3A%2F%2FQmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html%23header-%C4%85", http.StatusMovedPermanently, "/ipfs/QmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco/wiki/Foo_%c4%85%c4%99.html?filename=test-%c4%99.html"},
-			{"/ipns/?uri=ipns%3A%2F%2Fexample.com%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html", http.StatusMovedPermanently, "/ipns/example.com/wiki/Foo_%c4%85%c4%99.html?filename=test-%c4%99.html"},
+			{"/ipns/?uri=ipfs%3A%2F%2FQmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html%23header-%C4%85", http.StatusMovedPermanently, "/ipfs/QmXoypizjW3WknFiJnKLwHCnL72vedxjQkDDP1mXWo6uco/wiki/Foo_%C4%85%C4%99.html?filename=test-%C4%99.html"},
+			{"/ipns/?uri=ipns%3A%2F%2Fexample.com%2Fwiki%2FFoo_%C4%85%C4%99.html%3Ffilename%3Dtest-%C4%99.html", http.StatusMovedPermanently, "/ipns/example.com/wiki/Foo_%C4%85%C4%99.html?filename=test-%C4%99.html"},
 			{"/ipns?uri=ipns://" + cid, http.StatusMovedPermanently, "/ipns/" + cid},
 			{"/ipns/?uri=ipns://" + cid, http.StatusMovedPermanently, "/ipns/" + cid},
 			{"/ipns/?uri=ipfs://" + cid, http.StatusMovedPermanently, "/ipfs/" + cid},

diff --git a/gateway/handler.go b/gateway/handler.go
@@ -906,9 +906,10 @@ func handleProtocolHandlerRedirect(w http.ResponseWriter, r *http.Request, c *Co
 			webError(w, r, c, fmt.Errorf("uri query parameter scheme must be ipfs or ipns: %w", err), http.StatusBadRequest)
 			return true
 		}
-		path := u.Path
+
+		path := u.EscapedPath()
 		if u.RawQuery != "" { // preserve query if present
-			path = path + "?" + u.RawQuery
+			path += "?" + url.PathEscape(u.RawQuery)
 		}
 
 		redirectURL := gopath.Join("/", u.Scheme, u.Host, path)
@@ -989,7 +990,7 @@ func (i *handler) handleSuperfluousNamespace(w http.ResponseWriter, r *http.Requ
 	}
 
 	// Attempt to fix the superflous namespace
-	intendedPath, err := path.NewPath(strings.TrimPrefix(r.URL.Path, "/ipfs"))
+	intendedPath, err := path.NewPath(strings.TrimPrefix(r.URL.EscapedPath(), "/ipfs"))
 	if err != nil {
 		i.webError(w, r, fmt.Errorf("invalid ipfs path: %w", err), http.StatusBadRequest)
 		return true

diff --git a/gateway/handler_codec.go b/gateway/handler_codec.go
@@ -170,10 +170,15 @@ func (i *handler) serveCodecHTML(ctx context.Context, w http.ResponseWriter, r *
 		suffix := "/"
 		// preserve query parameters
 		if r.URL.RawQuery != "" {
-			suffix = suffix + "?" + r.URL.RawQuery
+			suffix = suffix + "?" + url.PathEscape(r.URL.RawQuery)
+		}
+		// Re-escape path instead of reusing RawPath to avod mix of lawer
+		// and upper hex that may come from RawPath.
+		if strings.ContainsRune(requestURI.RawPath, '%') {
+			requestURI.RawPath = ""
 		}
 		// /ipfs/cid/foo?bar must be redirected to /ipfs/cid/foo/?bar
-		redirectURL := requestURI.Path + suffix
+		redirectURL := requestURI.EscapedPath() + suffix
 		http.Redirect(w, r, redirectURL, http.StatusMovedPermanently)
 		return true
 	}

diff --git a/gateway/handler_unixfs_dir.go b/gateway/handler_unixfs_dir.go
@@ -47,11 +47,17 @@ func (i *handler) serveDirectory(ctx context.Context, w http.ResponseWriter, r *
 			suffix := "/"
 			// preserve query parameters
 			if r.URL.RawQuery != "" {
-				suffix = suffix + "?" + r.URL.RawQuery
+				suffix = suffix + "?" + url.PathEscape(r.URL.RawQuery)
+			}
+			// Re-escape path instead of reusing RawPath to avod mix of lawer
+			// and upper hex that may come from RawPath.
+			if strings.ContainsRune(requestURI.RawPath, '%') {
+				requestURI.RawPath = ""
 			}
 			// /ipfs/cid/foo?bar must be redirected to /ipfs/cid/foo/?bar
 			redirectURL := requestURI.EscapedPath() + suffix
 			rq.logger.Debugw("directory location moved permanently", "status", http.StatusMovedPermanently)
+
 			http.Redirect(w, r, redirectURL, http.StatusMovedPermanently)
 			return true
 		}