[Asterisk-code-review] chan sip: bigger buffers for headers, better failure mode (asterisk[master])

[Corey Farrell]

Corey Farrell has posted comments on this change.

Change subject: chan_sip: bigger buffers for headers, better failure mode
......................................................................


Patch Set 3: Code-Review-1

(1 comment)

> (1 comment)
 > 
 > > Did you plan on incorporating the feedback about not limiting
 > > things as Corey mentioned since the API easily allows it, or are
 > > you sticking to the enforced limit? As well does this close the
 > > issue like Corey mentioned?
 > 
 > It closes the issue, I'll add the Close:, I thought someone else
 > had to verify that part.
 > 
 > For the limit, I can remove it, if it's the way it should be, I was
 > extra paranoid.

You add the '#close' if you think it's appropriate.  When I (or someone else) is happy with this change we will give +1, that is verification of everything including the commit message.

https://gerrit.asterisk.org/#/c/2923/2/channels/chan_sip.c
File channels/chan_sip.c:

Line 14159: #define SIPHEADERMAX 1024
> I wanted to have some kind of limit, to guard against a possible DoS. The S
External input is limited by SIP_MAX_PACKET_SIZE (found in channels/sip/include/sip.h).  There is no need to limit the individual header size since it's already limited by the maximum packet size.  So my vote is to remove the SIPHEADERMAX define and just use 0 for the second parameter to ast_str_append throughout initreqprep.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1b68fcbddca6f6cc7d7a92fe1cb0d5430282b2b3
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Vasil Kolev <vasil.kolev at securax.org>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Vasil Kolev <vasil.kolev at securax.org>
Gerrit-HasComments: Yes