mirror of
https://github.com/fleetdm/fleet
synced 2026-05-23 17:08:53 +00:00
Add additional validation to mdmMicrosoftAuthEndpoint (#38147)
- [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) ## Testing - [x] Added/updated automated tests - [ ] QA'd all new/changed functionality manually
This commit is contained in:
parent
0c2b0d2039
commit
0e6c790803
4 changed files with 169 additions and 3 deletions
1
changes/fix-redirect-url-handling
Normal file
1
changes/fix-redirect-url-handling
Normal file
|
|
@ -0,0 +1 @@
|
|||
* Added additional validation to URL parameter for MS MDM auth endpoint
|
||||
|
|
@ -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,<script>alert(1)</script>",
|
||||
},
|
||||
{
|
||||
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()
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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,<script>alert(1)</script>",
|
||||
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"))
|
||||
|
|
|
|||
Loading…
Reference in a new issue