◐ Shell
clean mode source ↗

fix(site): escape appearance values in HTML output (#25804) (#26321) · coder/coder@77253bf

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

1414

"path/filepath"

1515

"strconv"

1616

"strings"

17+

"sync/atomic"

1718

"testing"

1819

"testing/fstest"

1920

"time"

@@ -24,6 +25,7 @@ import (

2425

"github.com/stretchr/testify/assert"

2526

"github.com/stretchr/testify/require"

262728+

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

2729

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

2830

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

2931

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

@@ -36,6 +38,81 @@ import (

3638

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

3739

)

384041+

type staticAppearanceFetcher struct {

42+

cfg codersdk.AppearanceConfig

43+

}

44+45+

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

46+

return f.cfg, nil

47+

}

48+49+

func TestInjectionAppearanceEscapesMetaAttributes(t *testing.T) {

50+

t.Parallel()

51+52+

const (

53+

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

54+

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

55+

)

56+57+

tests := []struct {

58+

name string

59+

authenticated bool

60+

}{

61+

{

62+

name: "unauthenticated",

63+

},

64+

{

65+

name: "authenticated",

66+

authenticated: true,

67+

},

68+

}

69+70+

for _, tt := range tests {

71+

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

72+

t.Parallel()

73+74+

siteFS := fstest.MapFS{

75+

"index.html": &fstest.MapFile{

76+

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

77+

},

78+

}

79+

db, _ := dbtestutil.NewDB(t)

80+

var appearanceFetcher atomic.Pointer[appearance.Fetcher]

81+

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

82+

ApplicationName: applicationName,

83+

LogoURL: logoURL,

84+

}})

85+

appearanceFetcher.Store(&fetcher)

86+

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

87+

Telemetry: telemetry.NewNoop(),

88+

Database: db,

89+

SiteFS: siteFS,

90+

AppearanceFetcher: &appearanceFetcher,

91+

})

92+93+

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

94+

if tt.authenticated {

95+

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

96+

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

97+

UserID: user.ID,

98+

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

99+

})

100+

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

101+

}

102+

rw := httptest.NewRecorder()

103+104+

handler.ServeHTTP(rw, r)

105+

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

106+

body := rw.Body.String()

107+108+

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

109+

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

110+

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

111+

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

112+

})

113+

}

114+

}

115+39116

func TestInjection(t *testing.T) {

40117

t.Parallel()

41118