Process content more safely

t2bot · Jul 9, 2024 · df6f38e · df6f38e
1 parent 51f24e9
commit df6f38e
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 20 deletions.
diff --git a/matrix/xmatrix.go b/matrix/xmatrix.go
@@ -7,7 +7,6 @@ import (
 	"net/http"
 	"strings"
 
-	"github.com/t2bot/matrix-media-repo/database"
 	"github.com/t2bot/matrix-media-repo/util"
 )
 
@@ -37,14 +36,14 @@ func ValidateXMatrixAuth(request *http.Request, expectNoContent bool) (string, e
 		uri = uri[:len(uri)-1]
 	}
 
-	err = ValidateXMatrixAuthHeader(request.Method, uri, &database.AnonymousJson{}, auths, keys, request.Host)
+	err = ValidateXMatrixAuthHeader(request.Method, uri, nil, auths, keys, request.Host)
 	if err != nil {
 		return "", err
 	}
 	return auths[0].Origin, nil
 }
 
-func ValidateXMatrixAuthHeader(requestMethod string, requestUri string, content any, headers []util.XMatrixAuth, originKeys ServerSigningKeys, destinationHost string) error {
+func ValidateXMatrixAuthHeader(requestMethod string, requestUri string, content []byte, headers []util.XMatrixAuth, originKeys ServerSigningKeys, destinationHost string) error {
 	if len(headers) == 0 {
 		return ErrNoXMatrixAuth
 	}
@@ -54,7 +53,9 @@ func ValidateXMatrixAuthHeader(requestMethod string, requestUri string, content
 		"uri":         requestUri,
 		"origin":      headers[0].Origin,
 		"destination": headers[0].Destination,
-		"content":     content,
+	}
+	if content != nil {
+		obj["content"] = content // we can't modify the content, so don't
 	}
 	canonical, err := util.EncodeCanonicalJson(obj)
 	if err != nil {
@@ -84,19 +85,15 @@ func ValidateXMatrixAuthHeader(requestMethod string, requestUri string, content
 	return nil
 }
 
-func CreateXMatrixHeader(origin string, destination string, requestMethod string, requestUri string, content any, key ed25519.PrivateKey, keyVersion string) (string, error) {
+func CreateXMatrixHeader(origin string, destination string, requestMethod string, requestUri string, content []byte, key ed25519.PrivateKey, keyVersion string) (string, error) {
 	obj := map[string]interface{}{
 		"method":      requestMethod,
 		"uri":         requestUri,
 		"origin":      origin,
 		"destination": destination,
 	}
 	if content != nil {
-		serializedContent, err := util.EncodeCanonicalJson(content)
-		if err != nil {
-			return "", err
-		}
-		obj["content"] = serializedContent
+		obj["content"] = content
 	}
 	canonical, err := util.EncodeCanonicalJson(obj)
 	if err != nil {

diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go
@@ -13,7 +13,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 	"github.com/t2bot/matrix-media-repo/common/config"
-	"github.com/t2bot/matrix-media-repo/database"
 	"github.com/t2bot/matrix-media-repo/homeserver_interop"
 	"github.com/t2bot/matrix-media-repo/matrix"
 	"github.com/t2bot/matrix-media-repo/test/test_internals"
@@ -127,7 +126,7 @@ func (s *MSC3916DownloadsSuite) TestFederationDownloads() {
 	assert.Equal(t, http.StatusUnauthorized, raw.StatusCode)
 
 	// Now add the X-Matrix auth and try again
-	header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", uri, &database.AnonymousJson{}, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion)
+	header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", uri, nil, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion)
 	assert.NoError(t, err)
 	remoteClient.AuthHeaderOverride = header
 	raw, err = remoteClient.DoRaw("GET", uri, nil, "", nil)
@@ -235,7 +234,7 @@ func (s *MSC3916DownloadsSuite) TestFederationProducesRedirects() {
 
 	// Verify the federation download *fails* when lacking auth
 	uri := fmt.Sprintf("/_matrix/federation/v1/media/download/%s", mediaId)
-	header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", uri, &database.AnonymousJson{}, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion)
+	header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", uri, nil, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion)
 	assert.NoError(t, err)
 	remoteClient.AuthHeaderOverride = header
 	raw, err := remoteClient.DoRaw("GET", uri, nil, "", nil)

diff --git a/test/msc3916_thumbnails_suite_test.go b/test/msc3916_thumbnails_suite_test.go
@@ -9,7 +9,6 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
-	"github.com/t2bot/matrix-media-repo/database"
 	"github.com/t2bot/matrix-media-repo/homeserver_interop"
 	"github.com/t2bot/matrix-media-repo/matrix"
 	"github.com/t2bot/matrix-media-repo/test/test_internals"
@@ -118,7 +117,7 @@ func (s *MSC3916ThumbnailsSuite) TestFederationThumbnails() {
 	assert.Equal(t, http.StatusUnauthorized, raw.StatusCode)
 
 	// Now add the X-Matrix auth and try again
-	header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", fmt.Sprintf("%s?%s", uri, qs.Encode()), &database.AnonymousJson{}, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion)
+	header, err := matrix.CreateXMatrixHeader(s.keyServer.PublicHostname, remoteClient.ServerName, "GET", fmt.Sprintf("%s?%s", uri, qs.Encode()), nil, s.keyServerKey.PrivateKey, s.keyServerKey.KeyVersion)
 	assert.NoError(t, err)
 	remoteClient.AuthHeaderOverride = header
 	raw, err = remoteClient.DoRaw("GET", uri, qs, "", nil)

diff --git a/test/xmatrix_header_test.go b/test/xmatrix_header_test.go
@@ -6,20 +6,21 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/t2bot/matrix-media-repo/common/config"
-	"github.com/t2bot/matrix-media-repo/database"
 	"github.com/t2bot/matrix-media-repo/matrix"
 	"github.com/t2bot/matrix-media-repo/util"
 )
 
 func TestXMatrixAuthHeader(t *testing.T) {
+	body := []byte(nil)
+
 	config.AddDomainForTesting("localhost", nil)
 
 	pub, priv, err := ed25519.GenerateKey(nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	header, err := matrix.CreateXMatrixHeader("localhost:8008", "localhost", "GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, priv, "0")
+	header, err := matrix.CreateXMatrixHeader("localhost:8008", "localhost", "GET", "/_matrix/media/v3/download/example.org/abc", body, priv, "0")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -31,19 +32,21 @@ func TestXMatrixAuthHeader(t *testing.T) {
 
 	keys := make(matrix.ServerSigningKeys)
 	keys["ed25519:0"] = pub
-	err = matrix.ValidateXMatrixAuthHeader("GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, auths, keys, "localhost")
+	err = matrix.ValidateXMatrixAuthHeader("GET", "/_matrix/media/v3/download/example.org/abc", body, auths, keys, "localhost")
 	assert.NoError(t, err)
 }
 
 func TestXMatrixAuthDestinationMismatch(t *testing.T) {
+	body := []byte(nil)
+
 	config.AddDomainForTesting("localhost", nil)
 
 	pub, priv, err := ed25519.GenerateKey(nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	header, err := matrix.CreateXMatrixHeader("localhost:8008", "localhost:1234", "GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, priv, "0")
+	header, err := matrix.CreateXMatrixHeader("localhost:8008", "localhost:1234", "GET", "/_matrix/media/v3/download/example.org/abc", body, priv, "0")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -55,6 +58,6 @@ func TestXMatrixAuthDestinationMismatch(t *testing.T) {
 
 	keys := make(matrix.ServerSigningKeys)
 	keys["ed25519:0"] = pub
-	err = matrix.ValidateXMatrixAuthHeader("GET", "/_matrix/media/v3/download/example.org/abc", &database.AnonymousJson{}, auths, keys, "localhost:1234")
+	err = matrix.ValidateXMatrixAuthHeader("GET", "/_matrix/media/v3/download/example.org/abc", body, auths, keys, "localhost:1234")
 	assert.ErrorIs(t, err, matrix.ErrWrongDestination)
 }