validate senders and rooms IDs in filters #325
Conversation
watson28
force-pushed
the
validate-senders-rooms-ids-filters
branch
from
8e8a7ed
to
71c621b
Compare
watson28
force-pushed
the
validate-senders-rooms-ids-filters
branch
2 times, most recently
from
4e69c96
to
b04c462
Compare
eventcontent.go
Outdated
|
|
||
| // Check if the room ID is a valid room ID. | ||
| func isValidRoomID(roomID string) bool { | ||
| return roomIDRegexp.MatchString((roomID)) |
There was a problem hiding this comment.
(roomID) should be sufficient here instead of ((roomID)).
eventcontent.go
Outdated
| IPV6AddressExpr := `[\d\x41-\x46\x61-\x66\:\.]{2,45}` | ||
| dnsNameExpr := `[\w\-\.]{1,255}` | ||
| portExpr := `\:\d{1,5}` | ||
| servernameExpr := fmt.Sprint(`(`, IPV4AddressExpr, `|[`, IPV6AddressExpr, `]|`, dnsNameExpr, `)(`, portExpr, `)?`) |
There was a problem hiding this comment.
For clarity can we use fmt.Sprintf here with a format string, like fmt.Sprintf(%s, %s, %s, %s, ...)?
Also is there a reason why IPV6AddressExpr is surrounded in more square brackets?
There was a problem hiding this comment.
Yes, that's part of the grammar specified at https://spec.matrix.org/v1.3/appendices/#server-name.
IP v6 addresses should be wrapped in square brackets when there is a prefix or suffix as part of a URI.
I forgot to scape those square brackets, so they are not interpreted as Regex character classes. But now it's fixed.
filter.go
Outdated
| if filter.Rooms != nil { | ||
| for _, roomID := range *filter.Rooms { | ||
| if !isValidRoomID(roomID) { | ||
| return errors.New("Bad room value. Must be in the form !localpart:domain") |
There was a problem hiding this comment.
It would be useful if the error here can return the faulty value, i.e. return fmt.Errorf("Bad room value %q: must be in form...", roomID)
| @@ -519,6 +542,10 @@ func (v *levelJSONValue) assignIfExists(to *int64) { | |||
|
|
|||
| // Check if the user ID is a valid user ID. | |||
| func isValidUserID(userID string) bool { | |||
| // TODO: Do we want to add anymore checks beyond checking the sigil and that it has a domain part? | |||
| return userID[0] == '@' && strings.IndexByte(userID, ':') != -1 | |||
| return userIDRegexp.MatchString(userID) | |||
There was a problem hiding this comment.
isValidUserID seems to be used in power level event auth, which means this is not safe (at least not before MSC2828). Right now it's possible to make user and room IDs with arbitrary unicode (spaces, emojis, everything). For example, @🐈️:maunium.net can be found in #dendrite-dev:matrix.org
I think filters aren't used for anything critical, but obviously validating them means you won't be able to filter user/room IDs containing non-ascii characters.
Signed-off-by: Jorge Florian <jafn28@gmail.com>
watson28
force-pushed
the
validate-senders-rooms-ids-filters
branch
from
b04c462
to
6c8bde4
Compare
Signed-off-by: Jorge Florian jafn28@gmail.com
Pull Request Checklist
Signed-off-by:
Jorge Florian <jafn28@gmail.com>This PR fixes: matrix-org/dendrite#2067