fix: Reject an upload URL whose host differs from the configured upload host

github/repos_releases.go

@@ -12,6 +12,7 @@ import (
 	"io"
 	"mime"
 	"net/http"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -504,6 +505,25 @@ func (s *RepositoriesService) UploadReleaseAssetFromRelease(
 	// so it works with Client.BaseURL path prefixes (e.g. "/api-v3/").
 	if !strings.HasPrefix(uploadURL, "http://") && !strings.HasPrefix(uploadURL, "https://") {
 		uploadURL = strings.TrimPrefix(uploadURL, "/")
+	} else {
+		// This helper is the one upload entry point whose URL comes from a server
+		// response rather than from the caller, and an absolute URL replaces the
+		// client's configured upload host entirely. Left unchecked, a response could
+		// name any host and receive the artifact together with the caller's
+		// Authorization header. Keep the upload on the host the client was
+		// configured with; that is uploads.github.com rather than api.github.com in
+		// the default configuration, which is why the comparison is against
+		// uploadURL and not baseURL.
+		u, err := url.Parse(uploadURL)
+		if err != nil {
+			return nil, nil, err
+		}
+		if !strings.EqualFold(u.Host, s.client.uploadURL.Host) {
+			return nil, nil, fmt.Errorf(
+				"upload URL host %q does not match the client's configured upload host %q",
+				u.Host, s.client.uploadURL.Host,
+			)
+		}
 	}
 
 	// addOptions will append name/label query params (same behavior as UploadReleaseAsset).

github/repos_releases_test.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/http/httptest"
 	"strings"
 	"testing"
 
@@ -850,6 +851,53 @@ func TestRepositoriesService_UploadReleaseAssetFromRelease_AbsoluteTemplate(t *t
 	}
 }
 
+func TestRepositoriesService_UploadReleaseAssetFromRelease_ForeignHostIsRejected(t *testing.T) {
+	t.Parallel()
+	client, _, _ := setup(t)
+
+	// A server that hands out an absolute upload URL naming a different host must not
+	// be able to redirect the upload - and therefore the caller's Authorization header
+	// and the artifact body - away from the host the client was configured with.
+	var leaked int
+	evil := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		leaked++
+		fmt.Fprint(w, `{"id":1}`)
+	}))
+	t.Cleanup(evil.Close)
+
+	body := []byte("private artifact\n")
+	reader := bytes.NewReader(body)
+	size := int64(len(body))
+
+	release := &RepositoryRelease{UploadURL: evil.URL + "/upload{?name,label}"}
+	ctx := t.Context()
+	_, _, err := client.Repositories.UploadReleaseAssetFromRelease(
+		ctx, release, &UploadOptions{Name: "n.txt"}, reader, size,
+	)
+	if err == nil {
+		t.Fatal("expected an error for an upload URL naming a foreign host, got nil")
+	}
+	if leaked != 0 {
+		t.Fatalf("upload reached the foreign host %d time(s); the token and body must never be sent there", leaked)
+	}
+}
+
+func TestRepositoriesService_UploadReleaseAssetFromRelease_MalformedUploadURL(t *testing.T) {
+	t.Parallel()
+	client, _, _ := setup(t)
+
+	// net/url rejects ASCII control characters, so a response naming such a URL must
+	// surface as an error rather than a panic or an upload to an unchecked host.
+	release := &RepositoryRelease{UploadURL: "https://uploads.github.com/\x7f/upload{?name,label}"}
+	ctx := t.Context()
+	_, _, err := client.Repositories.UploadReleaseAssetFromRelease(
+		ctx, release, &UploadOptions{Name: "n.txt"}, bytes.NewReader([]byte("x")), 1,
+	)
+	if err == nil {
+		t.Fatal("expected an error for an unparsable upload URL, got nil")
+	}
+}
+
 func TestRepositoriesService_UploadReleaseAssetFromRelease_NilRelease(t *testing.T) {
 	t.Parallel()
 	client, _, _ := setup(t)