[Asterisk-code-review] chan sip: Do not allow non-SP/HTAB between header key and co... (asterisk[11.25])

[Kevin Harwell]

Kevin Harwell has submitted this change and it was merged. ( https://gerrit.asterisk.org/4590 )

Change subject: chan_sip: Do not allow non-SP/HTAB between header key and colon.
......................................................................


chan_sip: Do not allow non-SP/HTAB between header key and colon.

RFC says SIP headers look like:

    HCOLON  =  *( SP / HTAB ) ":" SWS
    SWS     =  [LWS]                    ; sep whitespace
    LWS     =  [*WSP CRLF] 1*WSP        ; linear whitespace
    WSP     =  SP / HTAB                ; from rfc2234

chan_sip implemented this:

    HCOLON  =  *( LOWCTL / SP ) ":" SWS
    LOWCTL  = %x00-1F                   ; CTL without DEL

This discrepancy meant that SIP proxies in front of Asterisk with
chan_sip could pass on unknown headers with \x00-\x1F in them, which
would be treated by Asterisk as a different (known) header.  For
example, the "To\x01:" header would gladly be forwarded by some proxies
as irrelevant, but chan_sip would treat it as the relevant "To:" header.

Those relying on a SIP proxy to scrub certain headers could mistakenly
get unexpected and unvalidated data fed to Asterisk.

This change fixes so chan_sip only considers SP/HTAB as valid tokens
before the colon, making it agree on the headers with other speakers of
SIP.

ASTERISK-26433 #close
AST-2016-009

Change-Id: I78086fbc524ac733b8f7f78cb423c91075fd489b
(cherry picked from commit 26dd464dbd0ad7439bc29ce59ec55903d518ec6e)
---
M channels/chan_sip.c
1 file changed, 3 insertions(+), 5 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, approved; Verified
  Richard Mudgett: Looks good to me, but someone else must approve



diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 5310eb9..c45d6ca 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -8137,8 +8137,6 @@
 	 * one afterwards.  If you shouldn't do it, what absolute idiot decided it was
 	 * a good idea to say you can do it, and if you can do it, why in the hell would.
 	 * you say you shouldn't.
-	 * Anyways, pedanticsipchecking controls whether we allow spaces before ':',
-	 * and we always allow spaces after that for compatibility.
 	 */
 	const char *sname = find_alias(name, NULL);
 	int x, len = strlen(name), slen = (sname ? 1 : 0);
@@ -8151,10 +8149,10 @@
 		if (match || smatch) {
 			/* skip name */
 			const char *r = header + (match ? len : slen );
-			if (sip_cfg.pedanticsipchecking) {
-				r = ast_skip_blanks(r);
+			/* HCOLON has optional SP/HTAB; skip past those */
+			while (*r == ' ' || *r == '\t') {
+				++r;
 			}
-
 			if (*r == ':') {
 				*start = x+1;
 				return ast_skip_blanks(r+1);

-- 
To view, visit https://gerrit.asterisk.org/4590
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I78086fbc524ac733b8f7f78cb423c91075fd489b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11.25
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>