From 9fbce2154bc45da71bd5a1f89467d08b31fe4edd Mon Sep 17 00:00:00 2001 From: kolaente Date: Tue, 28 Jul 2026 09:28:46 +0200 Subject: [PATCH] fix(license): refuse redirects and use the SSRF-safe http client for checks The license servers are hardcoded, but the check client followed redirects without any policy and dialed without the SSRF guard, so a hijacked or poisoned license host could forward the license key to an internal address. Redirects are refused outright rather than capped: the check is a POST to a fixed JSON API that never redirects. --- pkg/license/check.go | 14 ++++++- pkg/license/check_test.go | 84 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 pkg/license/check_test.go diff --git a/pkg/license/check.go b/pkg/license/check.go index ad1cb7549..9df9cca23 100644 --- a/pkg/license/check.go +++ b/pkg/license/check.go @@ -20,6 +20,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "math/rand/v2" @@ -32,6 +33,7 @@ import ( "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/user" + "code.vikunja.io/api/pkg/utils" "code.vikunja.io/api/pkg/version" ) @@ -45,6 +47,8 @@ const ( requestTimeout = 10 * time.Second ) +var errRedirectNotFollowed = errors.New("license server redirect not followed") + // CheckRequest is the payload sent to the license server. type CheckRequest struct { LicenseKey string `json:"license_key"` @@ -144,8 +148,14 @@ func doRequest(serverURL string, body []byte) (*Response, error) { } req.Header.Set("Content-Type", "application/json") - client := &http.Client{} - resp, err := client.Do(req) //nolint:gosec // The URL is not user-controlled, it comes from hardcoded license server constants. + client := utils.NewSSRFSafeHTTPClient() + // The license servers are a fixed JSON API which never redirects. Following one would forward + // the license key to whatever host the redirect names, so refuse instead of capping the chain. + client.CheckRedirect = func(_ *http.Request, _ []*http.Request) error { + return errRedirectNotFollowed + } + + resp, err := client.Do(req) //nolint:gosec // SSRF protection is handled by the SSRF-safe client if err != nil { return nil, err } diff --git a/pkg/license/check_test.go b/pkg/license/check_test.go new file mode 100644 index 000000000..6df6b9358 --- /dev/null +++ b/pkg/license/check_test.go @@ -0,0 +1,84 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package license + +import ( + "io" + "net/http" + "net/http/httptest" + "testing" + + "code.vikunja.io/api/pkg/config" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDoRequestRefusesRedirects(t *testing.T) { + config.InitDefaultConfig() + config.OutgoingRequestsAllowNonRoutableIPs.Set("true") + defer config.OutgoingRequestsAllowNonRoutableIPs.Set("false") + + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"valid":true}`)) + })) + defer target.Close() + + redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL, http.StatusTemporaryRedirect) + })) + defer redirector.Close() + + resp, err := doRequest(redirector.URL, []byte(`{}`)) + require.Error(t, err) + assert.Nil(t, resp) + assert.ErrorIs(t, err, errRedirectNotFollowed) +} + +func TestDoRequestBlocksNonRoutableTarget(t *testing.T) { + config.InitDefaultConfig() + config.OutgoingRequestsAllowNonRoutableIPs.Set("false") + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"valid":true}`)) + })) + defer server.Close() + + resp, err := doRequest(server.URL, []byte(`{}`)) + require.Error(t, err) + assert.Nil(t, resp) + assert.NotErrorIs(t, err, errRedirectNotFollowed) +} + +func TestDoRequestSucceeds(t *testing.T) { + config.InitDefaultConfig() + config.OutgoingRequestsAllowNonRoutableIPs.Set("true") + defer config.OutgoingRequestsAllowNonRoutableIPs.Set("false") + + var gotBody []byte + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotBody, _ = io.ReadAll(r.Body) + _, _ = w.Write([]byte(`{"valid":true,"max_users":5}`)) + })) + defer server.Close() + + resp, err := doRequest(server.URL, []byte(`{"license_key":"abc"}`)) + require.NoError(t, err) + assert.True(t, resp.Valid) + assert.Equal(t, int64(5), resp.MaxUsers) + assert.JSONEq(t, `{"license_key":"abc"}`, string(gotBody)) +}