[bugfix] Sanitize incoming PropertyValue fields (#2722)

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

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

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": "<strong>cheeky</strong>",
+				"type": "PropertyValue",
+				"value": "<script>alert(\"teehee!\")</script>"
+			},
+			{
+				"name": "buy me coffee?",
+				"type": "PropertyValue",
+				"value": "<a href=\"https://example.org/some_link_to_my_ko_fi\">Right here!</a>"
+			},
+			{
+				"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(`<strong>cheeky</strong>`, fields[0].Name)
+	suite.Equal(`<script>alert("teehee!")</script>`, fields[0].Value)
+
+	// More or less OK field.
+	suite.Equal(`buy me coffee?`, fields[1].Name)
+	suite.Equal(`<a href="https://example.org/some_link_to_my_ko_fi">Right here!</a>`, 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(`<a href="https://example.org/some_link_to_my_ko_fi" rel="nofollow noreferrer noopener" target="_blank">Right here!</a>`, 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))