diff --git a/compose/service/record.go b/compose/service/record.go index 46f12212ec..b3eac94efa 100644 --- a/compose/service/record.go +++ b/compose/service/record.go @@ -50,6 +50,7 @@ type ( recordValuesSanitizer interface { Run(*types.Module, types.RecordValueSet) types.RecordValueSet + RunXSS(*types.Module, types.RecordValueSet) types.RecordValueSet } recordValuesValidator interface { @@ -233,6 +234,7 @@ func (svc record) lookup(ctx context.Context, namespaceID, moduleID uint64, look } r.SetModule(m) + r.Values = svc.sanitizer.RunXSS(m, r.Values) return nil }() @@ -312,6 +314,7 @@ func (svc record) Find(ctx context.Context, filter types.RecordFilter) (set type _ = set.Walk(func(r *types.Record) error { r.SetModule(m) + r.Values = svc.sanitizer.RunXSS(m, r.Values) return nil }) diff --git a/compose/service/values/sanitizer.go b/compose/service/values/sanitizer.go index 21c3601702..f86d567fa6 100644 --- a/compose/service/values/sanitizer.go +++ b/compose/service/values/sanitizer.go @@ -2,13 +2,16 @@ package values import ( "fmt" - "github.com/cortezaproject/corteza-server/pkg/expr" - "github.com/cortezaproject/corteza-server/pkg/logger" - "go.uber.org/zap" + "regexp" "strconv" "strings" "time" + "github.com/cortezaproject/corteza-server/pkg/expr" + "github.com/cortezaproject/corteza-server/pkg/logger" + "github.com/microcosm-cc/bluemonday" + "go.uber.org/zap" + "github.com/cortezaproject/corteza-server/compose/types" ) @@ -117,7 +120,7 @@ func (s sanitizer) Run(m *types.Module, vv types.RecordValueSet) (out types.Reco } // Per field type validators - switch strings.ToLower(f.Kind) { + switch kind { case "bool": v.Value = sBool(v.Value) @@ -127,6 +130,9 @@ func (s sanitizer) Run(m *types.Module, vv types.RecordValueSet) (out types.Reco case "number": v.Value = sNumber(v.Value, f.Options.Precision()) + case "string": + v.Value = sString(v.Value) + // Uncomment when they become relevant for sanitization //case "email": // v = s.sEmail(v, f, m) @@ -136,8 +142,6 @@ func (s sanitizer) Run(m *types.Module, vv types.RecordValueSet) (out types.Reco // v = s.sRecord(v, f, m) //case "select": // v = s.sSelect(v, f, m) - //case "string": - // v = s.sString(v, f, m) //case "url": // v = s.sUrl(v, f, m) //case "user": @@ -148,6 +152,29 @@ func (s sanitizer) Run(m *types.Module, vv types.RecordValueSet) (out types.Reco return } +func (s sanitizer) RunXSS(m *types.Module, vv types.RecordValueSet) types.RecordValueSet { + var ( + f *types.ModuleField + ) + + for _, v := range vv { + f = m.Fields.FindByName(v.Name) + if f == nil { + // Unknown field, + // if it is not handled before, + // sanitizer does not care about it + continue + } + + switch strings.ToLower(f.Kind) { + case "string": + v.Value = sString(v.Value) + } + } + + return vv +} + func sBool(v interface{}) string { switch c := v.(type) { case bool: @@ -258,6 +285,19 @@ func sNumber(num interface{}, p uint) string { return str } +// sString is used mostly to strip insecure html data +// from strings +func sString(str string) string { + // use standard html escaping policy + p := bluemonday.UGCPolicy() + + // match only colors for html editor elements on style attr + p.AllowAttrs("style").OnElements("span", "p") + p.AllowStyles("color").Matching(regexp.MustCompile("(?i)^#([0-9a-f]{3,4}|[0-9a-f]{6}|[0-9a-f]{8})$")).Globally() + + return p.Sanitize(str) +} + // sanitize casts value to field kind format func sanitize(f *types.ModuleField, v interface{}) string { switch strings.ToLower(f.Kind) { diff --git a/compose/service/values/sanitizer_test.go b/compose/service/values/sanitizer_test.go index 7491c682b7..036e6a8bc6 100644 --- a/compose/service/values/sanitizer_test.go +++ b/compose/service/values/sanitizer_test.go @@ -2,11 +2,12 @@ package values import ( "fmt" - "github.com/stretchr/testify/assert" "reflect" "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/cortezaproject/corteza-server/compose/types" ) @@ -160,6 +161,160 @@ func Test_sanitizer_Run(t *testing.T) { input: "42.040", output: "42.04", }, + { + name: "string escaping; html", + kind: "String", + options: map[string]interface{}{}, + input: "Title here", + output: "Title here", + }, + { + name: "string escaping; html a.href with javascript alert", + kind: "String", + options: map[string]interface{}{}, + input: `XSS`, + output: "XSS", + }, + { + name: "string escaping; a.href with javascript", + kind: "String", + options: map[string]interface{}{}, + input: `XSS`, + output: "XSS", + }, + { + name: "string escaping; script with script", + kind: "String", + options: map[string]interface{}{}, + input: `pt src="https://cortezaproject.org/script.js">`, + output: "pt src="https://cortezaproject.org/script.js">", + }, + { + name: "string escaping; script with a", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; meta with script", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; object", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; base href", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; script", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; div", + kind: "String", + options: map[string]interface{}{}, + input: `
`, + output: "
", + }, + { + name: "string escaping; frameset", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; iframe", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; meta", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; br", + kind: "String", + options: map[string]interface{}{}, + input: `
`, + output: "
", + }, + { + name: "string escaping; bgsound", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; input type image", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; style", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; link", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; html body onload event", + kind: "String", + options: map[string]interface{}{}, + input: ``, + output: "", + }, + { + name: "string escaping; xss element", + kind: "String", + options: map[string]interface{}{}, + input: `'';!--"=&{()}`, + output: "'';!--"=&{()}", + }, + { + name: "string escaping; xss element", + kind: "String", + options: map[string]interface{}{}, + input: `Hello there world.`, + output: "Hello there world.", + }, + { + name: "string escaping; xss element", + kind: "String", + options: map[string]interface{}{}, + input: `cortezaserver123`, + output: "cortezaserver123", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/tests/compose/record_test.go b/tests/compose/record_test.go index b4373959be..50a7470881 100644 --- a/tests/compose/record_test.go +++ b/tests/compose/record_test.go @@ -314,6 +314,66 @@ func TestRecordCreate_forbidenFields(t *testing.T) { End() } +func TestRecordCreate_xss(t *testing.T) { + h := newHelper(t) + h.clearRecords() + + h.allow(types.NamespaceRBACResource.AppendWildcard(), "read") + h.allow(types.ModuleRBACResource.AppendWildcard(), "read") + h.allow(types.ModuleRBACResource.AppendWildcard(), "record.create") + h.allow(types.ModuleRBACResource.AppendWildcard(), "record.update") + h.allow(types.ModuleRBACResource.AppendWildcard(), "record.read") + + var ( + ns = h.makeNamespace("some-namespace") + mod = h.makeModule(ns, "some-module", + &types.ModuleField{ + Kind: "String", + Name: "dummy", + }, + &types.ModuleField{ + Kind: "String", + Name: "dummyRichTextBox", + Options: map[string]interface{}{ + "useRichTextEditor": true, + }, + }, + ) + ) + + t.Run("create with rich text fields", func(t *testing.T) { + var ( + req = require.New(t) + + payload = struct { + Response *types.Record + }{} + + rec = &types.Record{ + Values: types.RecordValueSet{ + &types.RecordValue{Name: "dummyRichTextBox", Value: "test"}, + &types.RecordValue{Name: "dummy", Value: "simple-text"}, + }, + } + ) + + h.apiInit(). + Post(fmt.Sprintf("/namespace/%d/module/%d/record/", ns.ID, mod.ID)). + JSON(helpers.JSON(rec)). + Expect(t). + Status(http.StatusOK). + Assert(jsonpath.Present(`$.response.values[? @.name=="dummyRichTextBox"]`)). + Assert(jsonpath.Present(`$.response.values[? @.name=="dummy"]`)). + Assert(jsonpath.Present(`$.response.values[? @.value=="simple-text"]`)). + Assert(jsonpath.Present(`$.response.values[? @.value=="test"]`)). + End(). + JSON(&payload) + + req.NotNil(payload.Response) + req.NotZero(payload.Response.ID) + }) +} + func TestRecordCreateWithErrors(t *testing.T) { h := newHelper(t) h.clearRecords()