TEL-892: Overriding the To user when req.ToUserOverride is present - #795
TEL-892: Overriding the To user when req.ToUserOverride is present#795genseric-ghiro wants to merge 1 commit into
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if userOverride != "" { | ||
| su.User = userOverride | ||
| } |
There was a problem hiding this comment.
🟡 Replacement callee name is used without the checks applied to every other callee name
The callee name in the outgoing call's destination header is replaced (su.User = userOverride at pkg/sip/client.go:318-320) after all validation has already run, so a name containing address characters produces a malformed outgoing call header that the far end can reject.
Impact: Calls with such an override value can fail with a protocol error instead of a clear, early validation message.
Override bypasses the user validation done for every other path
Every other way of populating the user part is validated: the legacy path rejects users containing @ (pkg/sip/client.go:192-194) and the values path requires a non-empty user and rejects a host with a port (pkg/sip/client.go:245-258). The new override is applied directly to su.User with no checks, so a value like 333@other.com or one containing ;/> is serialized verbatim into the To header at pkg/sip/outbound.go:1158, producing an invalid header. Adding the same strings.Contains(userOverride, "@") guard (and rejecting other URI delimiters) would keep behavior consistent.
| if userOverride != "" { | |
| su.User = userOverride | |
| } | |
| if userOverride != "" { | |
| if strings.ContainsAny(userOverride, "@;<>") { | |
| return nil, fmt.Errorf("to user override should be a phone number or SIP user, not a full SIP URI") | |
| } | |
| su.User = userOverride | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Validation is done but at the protocol level, see here https://github.com/livekit/protocol/pull/1717/changes#diff-11cd5c4952372d3584a3e33299c5c6cf7fc9ff51868f798df87461c449014886R921-R928
| if userOverride != "" { | ||
| su.User = userOverride | ||
| } |
There was a problem hiding this comment.
🟨 Override value for the To header user is applied without any input validation
The new ToUserOverride value is written directly into the To header's user part (pkg/sip/client.go:318-320) without the sanity checks applied on all other code paths (the legacy path rejects users containing @ at pkg/sip/client.go:192-194). A value containing SIP URI delimiters (@, ;, <, >) is serialized verbatim into the outgoing INVITE To header (pkg/sip/outbound.go:1158), allowing header/URI manipulation of the emitted SIP message.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
same as above
nishadmusthafa
left a comment
nishadmusthafa
left a comment
There was a problem hiding this comment.
Yeah, if there's no way to bypass the protocol validation, I think we should be good.
2 participants
Summary
Touser whenreq.ToUserOverrideis present