From f487fc5d4be9175db571b5a573ea5d1f348f014a Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Mon, 4 Mar 2024 11:46:59 +0100 Subject: [PATCH] [bugfix] Sanitize incoming PropertyValue fields (#2722) --- internal/ap/interfaces.go | 6 +++ internal/ap/normalize.go | 89 +++++++++++++++++++++++++++++++++++ internal/ap/normalize_test.go | 49 +++++++++++++++++++ 3 files changed, 144 insertions(+) diff --git a/internal/ap/interfaces.go b/internal/ap/interfaces.go index fa8e8a338..05f6742cc 100644 --- a/internal/ap/interfaces.go +++ b/internal/ap/interfaces.go @@ -387,6 +387,12 @@ type WithName interface { SetActivityStreamsName(vocab.ActivityStreamsNameProperty) } +// WithValue represents an activity with SchemaValueProperty +type WithValue interface { + GetSchemaValue() vocab.SchemaValueProperty + SetSchemaValue(vocab.SchemaValueProperty) +} + // WithImage represents an activity with ActivityStreamsImageProperty type WithImage interface { GetActivityStreamsImage() vocab.ActivityStreamsImageProperty diff --git a/internal/ap/normalize.go b/internal/ap/normalize.go index 8e0a022c1..bef6d93b0 100644 --- a/internal/ap/normalize.go +++ b/internal/ap/normalize.go @@ -80,6 +80,7 @@ func NormalizeIncomingActivity(activity pub.Activity, rawJSON map[string]interfa if accountable, ok := ToAccountable(dataType); ok { // Normalize everything we can on the accountable. NormalizeIncomingSummary(accountable, rawData) + NormalizeIncomingFields(accountable, rawData) continue } } @@ -257,6 +258,64 @@ func NormalizeIncomingSummary(item WithSummary, rawJSON map[string]interface{}) item.SetActivityStreamsSummary(summaryProp) } +// NormalizeIncomingFields sanitizes any PropertyValue fields on the +// given WithAttachment interface, by removing html completely from +// the "name" field, and sanitizing dodgy HTML out of the "value" field. +func NormalizeIncomingFields(item WithAttachment, rawJSON map[string]interface{}) { + rawAttachments, ok := rawJSON["attachment"] + if !ok { + // No attachments in rawJSON. + return + } + + // Convert to slice if not already, + // so we can iterate through it. + var attachments []interface{} + if attachments, ok = rawAttachments.([]interface{}); !ok { + attachments = []interface{}{rawAttachments} + } + + attachmentProperty := item.GetActivityStreamsAttachment() + if attachmentProperty == nil { + // Nothing to do here. + return + } + + if l := attachmentProperty.Len(); l == 0 || l != len(attachments) { + // Mismatch between item and + // JSON, can't normalize. + return + } + + // Keep an index of where we are in the iter; + // we need this so we can modify the correct + // attachment, in case of multiples. + i := -1 + + for iter := attachmentProperty.Begin(); iter != attachmentProperty.End(); iter = iter.Next() { + i++ + + if !iter.IsSchemaPropertyValue() { + // Not interested. + continue + } + + pv := iter.GetSchemaPropertyValue() + if pv == nil { + // Odd. + continue + } + + rawPv, ok := attachments[i].(map[string]interface{}) + if !ok { + continue + } + + NormalizeIncomingName(pv, rawPv) + NormalizeIncomingValue(pv, rawPv) + } +} + // NormalizeIncomingName replaces the Name of the given item // with the raw 'name' value from the raw json object map. // @@ -289,6 +348,36 @@ func NormalizeIncomingName(item WithName, rawJSON map[string]interface{}) { item.SetActivityStreamsName(nameProp) } +// NormalizeIncomingValue replaces the Value of the given +// tem with the raw 'value' from the raw json object map. +// +// noop if there was no name in the json object map or the +// value was not a plain string. +func NormalizeIncomingValue(item WithValue, rawJSON map[string]interface{}) { + rawValue, ok := rawJSON["value"] + if !ok { + // No value in rawJSON. + return + } + + value, ok := rawValue.(string) + if !ok { + // Not interested in non-string name. + return + } + + // Value often contains links or + // mentions or other little snippets. + // Sanitize to HTML to allow these. + value = text.SanitizeToHTML(value) + + // Set normalized name property from the raw string; this + // will replace any existing value property on the item. + valueProp := streams.NewSchemaValueProperty() + valueProp.Set(value) + item.SetSchemaValue(valueProp) +} + // NormalizeIncomingOneOf normalizes all oneOf (if any) of the given // item, replacing the 'name' field of each oneOf with the raw 'name' // value from the raw json object map, and doing sanitization diff --git a/internal/ap/normalize_test.go b/internal/ap/normalize_test.go index 33b1f6ea6..3e4dc86f5 100644 --- a/internal/ap/normalize_test.go +++ b/internal/ap/normalize_test.go @@ -177,6 +177,23 @@ func (suite *NormalizeTestSuite) getAccountable() (vocab.ActivityStreamsPerson, "@context": "https://www.w3.org/ns/activitystreams", "id": "https://example.org/users/someone", "summary": "about: I'm a #Barbie #girl in a #Barbie #world\nLife in plastic, it's fantastic\nYou can brush my hair, undress me everywhere\nImagination, life is your creation\nI'm a blonde bimbo girl\nIn a fantasy world\nDress me up, make it tight\nI'm your dolly\nYou're my doll, rock and roll\nFeel the glamour in pink\nKiss me here, touch me there\nHanky panky", + "attachment": [ + { + "name": "cheeky", + "type": "PropertyValue", + "value": "" + }, + { + "name": "buy me coffee?", + "type": "PropertyValue", + "value": "Right here!" + }, + { + "name": "hello", + "type": "PropertyValue", + "value": "world" + } + ], "type": "Person" }`) @@ -405,6 +422,38 @@ Kiss me here, touch me there Hanky panky`, ap.ExtractSummary(accountable)) } +func (suite *NormalizeTestSuite) TestNormalizeAccountableFields() { + accountable, rawAccount := suite.getAccountable() + fields := ap.ExtractFields(accountable) + + // Dodgy field. + suite.Equal(`cheeky`, fields[0].Name) + suite.Equal(``, fields[0].Value) + + // More or less OK field. + suite.Equal(`buy me coffee?`, fields[1].Name) + suite.Equal(`Right here!`, fields[1].Value) + + // Fine field. + suite.Equal(`hello`, fields[2].Name) + suite.Equal(`world`, fields[2].Value) + + // Normalize 'em. + ap.NormalizeIncomingFields(accountable, rawAccount) + + // Dodgy field should be removed. + fields = ap.ExtractFields(accountable) + suite.Len(fields, 2) + + // More or less OK field is now very OK. + suite.Equal(`buy me coffee?`, fields[0].Name) + suite.Equal(`Right here!`, fields[0].Value) + + // Fine field continues to be fine. + suite.Equal(`hello`, fields[1].Name) + suite.Equal(`world`, fields[1].Value) +} + func (suite *NormalizeTestSuite) TestNormalizeStatusableSummary() { statusable, rawAccount := suite.getStatusableWithWeirdSummaryAndName() suite.Equal(`warning: #WEIRD%20%23SUMMARY%20;;;;a;;a;asv%20%20%20%20khop8273987(*%5E&%5E)`, ap.ExtractSummary(statusable))