diff --git a/changes/fix-redirect-url-handling b/changes/fix-redirect-url-handling
new file mode 100644
index 0000000000..8868298686
--- /dev/null
+++ b/changes/fix-redirect-url-handling
@@ -0,0 +1 @@
+* Added additional validation to URL parameter for MS MDM auth endpoint
diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go
index 59c0b30c84..afe081ef50 100644
--- a/server/service/integration_mdm_test.go
+++ b/server/service/integration_mdm_test.go
@@ -7917,7 +7917,8 @@ func (s *integrationMDMTestSuite) TestValidGetAuthRequest() {
// Checking response content
resContent := string(resBytes)
require.Contains(t, resContent, "inputToken.name = 'wresult'")
- require.Contains(t, resContent, "form.action = \"ms-app://windows.immersivecontrolpanel\"")
+ // we expect the URL to be escaped
+ require.Contains(t, resContent, `form.action = "ms-app:\/\/windows.immersivecontrolpanel"`)
require.Contains(t, resContent, "performPost()")
// Getting token content
@@ -7939,6 +7940,79 @@ func (s *integrationMDMTestSuite) TestInvalidGetAuthRequest() {
require.Contains(t, resContent, "forbidden")
}
+func (s *integrationMDMTestSuite) TestAppruValidationInGetAuthRequest() {
+ t := s.T()
+
+ // Test cases with invalid appru values that should bail due to exiting before auth check
+ invalidAppruCases := []struct {
+ name string
+ appru string
+ }{
+ {
+ name: "javascript injection",
+ appru: "%3Bfor%20(var%20key%20in%20localStorage)%7B%20alert(key)%7D%3B%2F%2F",
+ },
+ {
+ name: "javascript protocol",
+ appru: "javascript:alert(1)",
+ },
+ {
+ name: "data URI",
+ appru: "data:text/html,",
+ },
+ {
+ name: "empty scheme",
+ appru: "://example.com",
+ },
+ {
+ name: "plain text",
+ appru: "not-a-url",
+ },
+ }
+
+ for _, tc := range invalidAppruCases {
+ t.Run(tc.name, func(t *testing.T) {
+ targetEndpointURL := microsoft_mdm.MDE2AuthPath + "?appru=" + tc.appru + "&login_hint=demo%40example.com"
+ resp := s.DoRaw("GET", targetEndpointURL, nil, http.StatusInternalServerError)
+
+ resBytes, err := io.ReadAll(resp.Body)
+ resContent := string(resBytes)
+ require.NoError(t, err)
+ require.NotEmpty(t, resBytes)
+ require.Contains(t, resContent, "forbidden")
+
+ resp.Body.Close()
+ })
+ }
+
+ // Also verify valid URLs still work
+ validAppruCases := []struct {
+ name string
+ appru string
+ }{
+ {
+ name: "ms-app scheme",
+ appru: "ms-app%3A%2F%2Fwindows.immersivecontrolpanel",
+ },
+ {
+ name: "https scheme",
+ appru: "https%3A%2F%2Fexample.com%2Fcallback",
+ },
+ {
+ name: "http scheme",
+ appru: "http%3A%2F%2Flocalhost%2Fcallback",
+ },
+ }
+
+ for _, tc := range validAppruCases {
+ t.Run(tc.name, func(t *testing.T) {
+ targetEndpointURL := microsoft_mdm.MDE2AuthPath + "?appru=" + tc.appru + "&login_hint=demo%40example.com"
+ resp := s.DoRaw("GET", targetEndpointURL, nil, http.StatusOK)
+ resp.Body.Close()
+ })
+ }
+}
+
func (s *integrationMDMTestSuite) TestValidGetTOC() {
t := s.T()
diff --git a/server/service/microsoft_mdm.go b/server/service/microsoft_mdm.go
index 1d51745f3e..7e42ee8f30 100644
--- a/server/service/microsoft_mdm.go
+++ b/server/service/microsoft_mdm.go
@@ -11,13 +11,14 @@ import (
"errors"
"fmt"
"html"
+ "html/template"
"io"
"net"
"net/http"
"net/url"
+ "slices"
"strconv"
"strings"
- "text/template"
"time"
"github.com/fleetdm/fleet/v4/pkg/fleetdbase"
@@ -409,7 +410,7 @@ func NewSoapFault(errorType string, origMessage int, errorMessage error) mdm_typ
}
}
-// getSTSAuthContent Retuns STS auth content
+// getSTSAuthContent Returns STS auth content
func getSTSAuthContent(data string) mdm_types.Errorer {
return MDMAuthContainer{
Data: &data,
@@ -777,6 +778,17 @@ func mdmMicrosoftDiscoveryEndpoint(ctx context.Context, request interface{}, svc
}, nil
}
+// isValidAppru validates that appru is a valid URL with an allowed scheme.
+// It returns true if appru is a valid URL with http, https, or ms-app scheme.
+func isValidAppru(appru string) bool {
+ parsed, err := url.Parse(appru)
+ if err != nil {
+ return false
+ }
+
+ return slices.Contains([]string{"http", "https", "ms-app"}, parsed.Scheme)
+}
+
// mdmMicrosoftAuthEndpoint handles the Security Token Service (STS) implementation
func mdmMicrosoftAuthEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (mdm_types.Errorer, error) {
params := request.(*SoapRequestContainer).Params
@@ -793,6 +805,11 @@ func mdmMicrosoftAuthEndpoint(ctx context.Context, request interface{}, svc flee
return getSTSAuthContent(""), errors.New("expected STS params are empty")
}
+ // Validate that appru is a valid URL
+ if !isValidAppru(appru) {
+ return getSTSAuthContent(""), fmt.Errorf("non-URL appru parameter attempted: %q", appru)
+ }
+
// Getting the STS endpoint HTML content
stsAuthContent, err := svc.GetMDMMicrosoftSTSAuthResponse(ctx, appru, loginHint)
if err != nil {
diff --git a/server/service/microsoft_mdm_test.go b/server/service/microsoft_mdm_test.go
index bdd888b5a1..055d830783 100644
--- a/server/service/microsoft_mdm_test.go
+++ b/server/service/microsoft_mdm_test.go
@@ -14,6 +14,7 @@ import (
"github.com/fleetdm/fleet/v4/server/mdm/microsoft/syncml"
"github.com/fleetdm/fleet/v4/server/mock"
"github.com/go-kit/log"
+ "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -35,6 +36,79 @@ func NewSoapRequest(request []byte) (fleet.SoapRequest, error) {
return req, nil
}
+func TestIsValidAppruURL(t *testing.T) {
+ tests := []struct {
+ name string
+ appru string
+ expected bool
+ }{
+ // Valid URLs
+ {
+ name: "valid ms-app scheme",
+ appru: "ms-app://windows.immersivecontrolpanel",
+ expected: true,
+ },
+ {
+ name: "valid https scheme",
+ appru: "https://example.com/callback",
+ expected: true,
+ },
+ {
+ name: "valid http scheme",
+ appru: "http://localhost/callback",
+ expected: true,
+ },
+ // Invalid URLs - XSS attempts
+ {
+ name: "javascript injection",
+ appru: ";for (var key in localStorage){ alert(key)};//",
+ expected: false,
+ },
+ {
+ name: "javascript protocol",
+ appru: "javascript:alert(1)",
+ expected: false,
+ },
+ {
+ name: "data URI",
+ appru: "data:text/html,",
+ expected: false,
+ },
+ {
+ name: "empty scheme",
+ appru: "://example.com",
+ expected: false,
+ },
+ {
+ name: "plain text",
+ appru: "not-a-url",
+ expected: false,
+ },
+ {
+ name: "empty string",
+ appru: "",
+ expected: false,
+ },
+ {
+ name: "file scheme",
+ appru: "file:///etc/passwd",
+ expected: false,
+ },
+ {
+ name: "ftp scheme",
+ appru: "ftp://example.com",
+ expected: false,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ result := isValidAppru(tc.appru)
+ assert.Equal(t, tc.expected, result)
+ })
+ }
+}
+
func TestValidSoapResponse(t *testing.T) {
relatesTo := "urn:uuid:0d5a1441-5891-453b-becf-a2e5f6ea3749"
soapFaultMsg := NewSoapFault(syncml.SoapErrorAuthentication, fleet.MDEDiscovery, errors.New("test"))