◐ Shell
clean mode source ↗

fix: escape appearance values in HTML output (#25804) (#26246) · coder/coder@74f08d1

@@ -14,6 +14,7 @@ import (

1414

"path/filepath"

1515

"strconv"

1616

"strings"

17+

"sync/atomic"

1718

"testing"

1819

"testing/fstest"

1920

"time"

@@ -26,6 +27,7 @@ import (

2627

"github.com/stretchr/testify/require"

2728

"golang.org/x/exp/maps"

282930+

"github.com/coder/coder/v2/coderd/appearance"

2931

"github.com/coder/coder/v2/coderd/database"

3032

"github.com/coder/coder/v2/coderd/database/db2sdk"

3133

"github.com/coder/coder/v2/coderd/database/dbgen"

@@ -39,6 +41,82 @@ import (

3941

"github.com/coder/coder/v2/testutil"

4042

)

414344+

type staticAppearanceFetcher struct {

45+

cfg codersdk.AppearanceConfig

46+

}

47+48+

func (f staticAppearanceFetcher) Fetch(context.Context) (codersdk.AppearanceConfig, error) {

49+

return f.cfg, nil

50+

}

51+52+

func TestInjectionAppearanceEscapesMetaAttributes(t *testing.T) {

53+

t.Parallel()

54+55+

const (

56+

applicationName = `Coder"><script>alert(1)</script>`

57+

logoURL = `https://example.com/logo.png"><img src=x onerror=alert(1)>`

58+

)

59+60+

tests := []struct {

61+

name string

62+

authenticated bool

63+

}{

64+

{

65+

name: "unauthenticated",

66+

},

67+

{

68+

name: "authenticated",

69+

authenticated: true,

70+

},

71+

}

72+73+

for _, tt := range tests {

74+

t.Run(tt.name, func(t *testing.T) {

75+

t.Parallel()

76+77+

siteFS := fstest.MapFS{

78+

"index.html": &fstest.MapFile{

79+

Data: []byte(`<meta name="application-name" content="{{ .ApplicationName }}" /><meta property="logo-url" content="{{ .LogoURL }}" />`),

80+

},

81+

}

82+

db, _ := dbtestutil.NewDB(t)

83+

var appearanceFetcher atomic.Pointer[appearance.Fetcher]

84+

fetcher := appearance.Fetcher(staticAppearanceFetcher{cfg: codersdk.AppearanceConfig{

85+

ApplicationName: applicationName,

86+

LogoURL: logoURL,

87+

}})

88+

appearanceFetcher.Store(&fetcher)

89+

handler, err := site.New(&site.Options{

90+

Telemetry: telemetry.NewNoop(),

91+

Database: db,

92+

SiteFS: siteFS,

93+

AppearanceFetcher: &appearanceFetcher,

94+

})

95+

require.NoError(t, err)

96+97+

r := httptest.NewRequest("GET", "/", nil)

98+

if tt.authenticated {

99+

user := dbgen.User(t, db, database.User{})

100+

_, token := dbgen.APIKey(t, db, database.APIKey{

101+

UserID: user.ID,

102+

ExpiresAt: time.Now().Add(time.Hour),

103+

})

104+

r.Header.Set(codersdk.SessionTokenHeader, token)

105+

}

106+

rw := httptest.NewRecorder()

107+108+

handler.ServeHTTP(rw, r)

109+

require.Equal(t, http.StatusOK, rw.Code)

110+

body := rw.Body.String()

111+112+

require.True(t, strings.Contains(body, html.EscapeString(applicationName)), "application name must be HTML escaped")

113+

require.True(t, strings.Contains(body, html.EscapeString(logoURL)), "logo URL must be HTML escaped")

114+

require.False(t, strings.Contains(body, applicationName), "raw application name must not be rendered")

115+

require.False(t, strings.Contains(body, logoURL), "raw logo URL must not be rendered")

116+

})

117+

}

118+

}

119+42120

func TestInjection(t *testing.T) {

43121

t.Parallel()

44122