fix(caldav): escape user-controlled strings per RFC 5545 in VCALENDAR output
Task titles, UIDs, descriptions, categories, organizer usernames, alarm descriptions, relation UIDs, and the calendar name were concatenated raw into the VCALENDAR text. A task title containing CR/LF could plant new iCalendar properties (ATTACH, X-INJECTED, VALARM, etc.) that CalDAV clients would parse as legitimate calendar data. Introduce escapeICalText, which escapes backslash, CR/LF, semicolon, and comma per RFC 5545 §3.3.11, and apply it at every sink in ParseTodos, ParseAlarms, and ParseRelations. Each Category is escaped individually; the comma that joins categories is the literal list delimiter and stays unescaped. The now-redundant regexp-based LF handling in the DESCRIPTION branch is removed. getCaldavColor is hardened at the same output boundary: non-hex characters are stripped before interpolation so CR/LF in a crafted color string cannot inject new iCal property lines, closing a gap where upstream HexColor validation only bounds length and does not reject control characters. Fixes GHSA-2g7h-7rqr-9p4r.
This commit is contained in:
kolaente
kolaente
parent
fc216c38af
commit
aa2b8c43f1
|
|
@ -17,7 +17,6 @@
|
|||
package caldav
|
||||
|
||||
import (
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
|
@ -30,6 +29,19 @@ import (
|
|||
// DateFormat is the caldav date format
|
||||
const DateFormat = `20060102T150405`
|
||||
|
||||
// escapeICalText escapes a string for use inside an RFC 5545 §3.3.11 TEXT
|
||||
// property value. Backslash MUST be replaced first so the backslashes this
|
||||
// function inserts are not themselves re-escaped.
|
||||
func escapeICalText(s string) string {
|
||||
s = strings.ReplaceAll(s, "\r\n", "\n")
|
||||
s = strings.ReplaceAll(s, "\r", "\n")
|
||||
s = strings.ReplaceAll(s, `\`, `\\`)
|
||||
s = strings.ReplaceAll(s, "\n", `\n`)
|
||||
s = strings.ReplaceAll(s, `;`, `\;`)
|
||||
s = strings.ReplaceAll(s, `,`, `\,`)
|
||||
return s
|
||||
}
|
||||
|
||||
// Todo holds a single VTODO
|
||||
type Todo struct {
|
||||
// Required
|
||||
|
|
@ -77,16 +89,25 @@ type Config struct {
|
|||
Color string
|
||||
}
|
||||
|
||||
// getCaldavColor sanitizes color at the output boundary (non-hex chars
|
||||
// dropped) so upstream HexColor validation gaps cannot lead to iCal
|
||||
// property injection via CR/LF in a crafted color string.
|
||||
func getCaldavColor(color string) (caldavcolor string) {
|
||||
color = strings.TrimPrefix(color, "#")
|
||||
|
||||
var b strings.Builder
|
||||
b.Grow(len(color))
|
||||
for _, r := range color {
|
||||
if (r >= '0' && r <= '9') || (r >= 'a' && r <= 'f') || (r >= 'A' && r <= 'F') {
|
||||
b.WriteRune(r)
|
||||
}
|
||||
}
|
||||
color = b.String()
|
||||
if color == "" {
|
||||
return ""
|
||||
}
|
||||
|
||||
if !strings.HasPrefix(color, "#") {
|
||||
color = "#" + color
|
||||
}
|
||||
|
||||
color += "FF"
|
||||
color = "#" + color + "FF"
|
||||
|
||||
return `
|
||||
X-APPLE-CALENDAR-COLOR:` + color + `
|
||||
|
|
@ -131,7 +152,7 @@ func ParseTodos(config *Config, todos []*Todo) (caldavtodos string) {
|
|||
caldavtodos = `BEGIN:VCALENDAR
|
||||
VERSION:2.0
|
||||
X-PUBLISHED-TTL:PT4H
|
||||
X-WR-CALNAME:` + config.Name + `
|
||||
X-WR-CALNAME:` + escapeICalText(config.Name) + `
|
||||
PRODID:-//` + config.ProdID + `//EN` + getCaldavColor(config.Color)
|
||||
|
||||
for _, t := range todos {
|
||||
|
|
@ -141,9 +162,9 @@ PRODID:-//` + config.ProdID + `//EN` + getCaldavColor(config.Color)
|
|||
|
||||
caldavtodos += `
|
||||
BEGIN:VTODO
|
||||
UID:` + t.UID + `
|
||||
UID:` + escapeICalText(t.UID) + `
|
||||
DTSTAMP:` + makeCalDavTimeFromTimeStamp(t.Timestamp) + `
|
||||
SUMMARY:` + t.Summary + getCaldavColor(t.Color)
|
||||
SUMMARY:` + escapeICalText(t.Summary) + getCaldavColor(t.Color)
|
||||
|
||||
if t.Start.Unix() > 0 {
|
||||
caldavtodos += `
|
||||
|
|
@ -158,10 +179,8 @@ DURATION:PT` + formatDuration(t.Duration)
|
|||
DTEND:` + makeCalDavTimeFromTimeStamp(t.End)
|
||||
}
|
||||
if t.Description != "" {
|
||||
re := regexp.MustCompile(`\r?\n`)
|
||||
formattedDescription := re.ReplaceAllString(t.Description, "\\n")
|
||||
caldavtodos += `
|
||||
DESCRIPTION:` + formattedDescription
|
||||
DESCRIPTION:` + escapeICalText(t.Description)
|
||||
}
|
||||
if t.Completed.Unix() > 0 {
|
||||
caldavtodos += `
|
||||
|
|
@ -170,7 +189,7 @@ STATUS:COMPLETED`
|
|||
}
|
||||
if t.Organizer != nil {
|
||||
caldavtodos += `
|
||||
ORGANIZER;CN=:` + t.Organizer.Username
|
||||
ORGANIZER;CN=:` + escapeICalText(t.Organizer.Username)
|
||||
}
|
||||
|
||||
if t.DueDate.Unix() > 0 {
|
||||
|
|
@ -200,8 +219,12 @@ RRULE:FREQ=` + freq + `;INTERVAL=` + strconv.FormatInt(interval, 10)
|
|||
}
|
||||
|
||||
if len(t.Categories) > 0 {
|
||||
escapedCategories := make([]string, len(t.Categories))
|
||||
for i, c := range t.Categories {
|
||||
escapedCategories[i] = escapeICalText(c)
|
||||
}
|
||||
caldavtodos += `
|
||||
CATEGORIES:` + strings.Join(t.Categories, ",")
|
||||
CATEGORIES:` + strings.Join(escapedCategories, ",")
|
||||
}
|
||||
|
||||
caldavtodos += `
|
||||
|
|
@ -239,7 +262,7 @@ TRIGGER;VALUE=DATE-TIME:` + makeCalDavTimeFromTimeStamp(a.Time)
|
|||
}
|
||||
caldavalarms += `
|
||||
ACTION:DISPLAY
|
||||
DESCRIPTION:` + a.Description + `
|
||||
DESCRIPTION:` + escapeICalText(a.Description) + `
|
||||
END:VALARM`
|
||||
}
|
||||
return caldavalarms
|
||||
|
|
@ -280,7 +303,7 @@ RELATED-TO;RELTYPE=CHILD:`
|
|||
RELATED-TO:`
|
||||
}
|
||||
|
||||
caldavrelatedtos += r.UID
|
||||
caldavrelatedtos += escapeICalText(r.UID)
|
||||
}
|
||||
|
||||
return caldavrelatedtos
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ import (
|
|||
"code.vikunja.io/api/pkg/models"
|
||||
|
||||
"code.vikunja.io/api/pkg/config"
|
||||
"code.vikunja.io/api/pkg/user"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
|
|
@ -365,6 +366,66 @@ LAST-MODIFIED:00010101T000000Z
|
|||
RELATED-TO;RELTYPE=PARENT:parentuid
|
||||
RELATED-TO;RELTYPE=CHILD:subtaskuid
|
||||
END:VTODO
|
||||
END:VCALENDAR`,
|
||||
},
|
||||
{
|
||||
name: "GHSA-2g7h-7rqr-9p4r: CRLF injection in Summary is escaped",
|
||||
args: args{
|
||||
config: &Config{
|
||||
Name: "test",
|
||||
ProdID: "RandomProdID which is not random",
|
||||
},
|
||||
todos: []*Todo{
|
||||
{
|
||||
Summary: "Meeting\r\nATTACH:https://evil.com/malware.exe\r\nX-INJECTED:pwned",
|
||||
UID: "randommduid",
|
||||
Timestamp: time.Unix(1543626724, 0).In(config.GetTimeZone()),
|
||||
},
|
||||
},
|
||||
},
|
||||
wantCaldavtasks: `BEGIN:VCALENDAR
|
||||
VERSION:2.0
|
||||
X-PUBLISHED-TTL:PT4H
|
||||
X-WR-CALNAME:test
|
||||
PRODID:-//RandomProdID which is not random//EN
|
||||
BEGIN:VTODO
|
||||
UID:randommduid
|
||||
DTSTAMP:20181201T011204Z
|
||||
SUMMARY:Meeting\nATTACH:https://evil.com/malware.exe\nX-INJECTED:pwned
|
||||
LAST-MODIFIED:00010101T000000Z
|
||||
END:VTODO
|
||||
END:VCALENDAR`,
|
||||
},
|
||||
{
|
||||
name: "GHSA-2g7h-7rqr-9p4r: semicolons and commas in Summary are escaped",
|
||||
args: args{
|
||||
config: &Config{
|
||||
Name: "te;st,ed",
|
||||
ProdID: "RandomProdID which is not random",
|
||||
},
|
||||
todos: []*Todo{
|
||||
{
|
||||
Summary: "a;b,c\\d",
|
||||
UID: "randommduid",
|
||||
Timestamp: time.Unix(1543626724, 0).In(config.GetTimeZone()),
|
||||
Categories: []string{"lab;el1", "lab,el2"},
|
||||
Organizer: &user.User{Username: "al;ic,e"},
|
||||
},
|
||||
},
|
||||
},
|
||||
wantCaldavtasks: `BEGIN:VCALENDAR
|
||||
VERSION:2.0
|
||||
X-PUBLISHED-TTL:PT4H
|
||||
X-WR-CALNAME:te\;st\,ed
|
||||
PRODID:-//RandomProdID which is not random//EN
|
||||
BEGIN:VTODO
|
||||
UID:randommduid
|
||||
DTSTAMP:20181201T011204Z
|
||||
SUMMARY:a\;b\,c\\d
|
||||
ORGANIZER;CN=:al\;ic\,e
|
||||
CATEGORIES:lab\;el1,lab\,el2
|
||||
LAST-MODIFIED:00010101T000000Z
|
||||
END:VTODO
|
||||
END:VCALENDAR`,
|
||||
},
|
||||
}
|
||||
|
|
@ -376,3 +437,54 @@ END:VCALENDAR`,
|
|||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetCaldavColor(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
in string
|
||||
want string
|
||||
}{
|
||||
{"empty", "", ""},
|
||||
{"plain hex", "ff8800", "\nX-APPLE-CALENDAR-COLOR:#ff8800FF\nX-OUTLOOK-COLOR:#ff8800FF\nX-FUNAMBOL-COLOR:#ff8800FF\nCOLOR:#ff8800FF"},
|
||||
{"leading hash", "#ff8800", "\nX-APPLE-CALENDAR-COLOR:#ff8800FF\nX-OUTLOOK-COLOR:#ff8800FF\nX-FUNAMBOL-COLOR:#ff8800FF\nCOLOR:#ff8800FF"},
|
||||
{"mixed case", "AaBbCc", "\nX-APPLE-CALENDAR-COLOR:#AaBbCcFF\nX-OUTLOOK-COLOR:#AaBbCcFF\nX-FUNAMBOL-COLOR:#AaBbCcFF\nCOLOR:#AaBbCcFF"},
|
||||
{"CRLF injection stripped", "a\r\nB", "\nX-APPLE-CALENDAR-COLOR:#aBFF\nX-OUTLOOK-COLOR:#aBFF\nX-FUNAMBOL-COLOR:#aBFF\nCOLOR:#aBFF"},
|
||||
{"property injection stripped", "ff\r\nATTACH:https://evil.com", "\nX-APPLE-CALENDAR-COLOR:#ffAACecFF\nX-OUTLOOK-COLOR:#ffAACecFF\nX-FUNAMBOL-COLOR:#ffAACecFF\nCOLOR:#ffAACecFF"},
|
||||
{"non-hex chars dropped entirely", "zz!@#", ""},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := getCaldavColor(tt.in)
|
||||
assert.Equal(t, tt.want, got)
|
||||
// No output may ever contain CR or LF inside a property value.
|
||||
assert.NotContains(t, got, "\r")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestEscapeICalText(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
in string
|
||||
want string
|
||||
}{
|
||||
{"empty", "", ""},
|
||||
{"plain ASCII", "Hello World", "Hello World"},
|
||||
{"backslash", `a\b`, `a\\b`},
|
||||
{"semicolon", "a;b", `a\;b`},
|
||||
{"comma", "a,b", `a\,b`},
|
||||
{"newline LF", "a\nb", `a\nb`},
|
||||
{"carriage return LF pair", "a\r\nb", `a\nb`},
|
||||
{"lone carriage return", "a\rb", `a\nb`},
|
||||
{"multiple specials", `a\;b,c` + "\n" + "d", `a\\\;b\,c\nd`},
|
||||
{"backslash-n in source stays separate", `a\nb`, `a\\nb`},
|
||||
{"advisory PoC (CRLF + ATTACH)", "Meeting\r\nATTACH:https://evil.com/malware.exe", `Meeting\nATTACH:https://evil.com/malware.exe`},
|
||||
{"colon is NOT escaped (not a TEXT special)", "a:b", "a:b"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := escapeICalText(tt.in)
|
||||
assert.Equal(t, tt.want, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue