[Asterisk-code-review] ASTERISK-24601 Include LOCAL/REMOTE tags in BLF NOTIFY XML Patch Orig... (asterisk[16])

[Joshua Colp]

Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/15763 )

Change subject: ASTERISK-24601 Include LOCAL/REMOTE tags in BLF NOTIFY XML Patch Originally submitted by Joshua Elson Modified by Joseph Nadiv and Sean Bright
......................................................................


Patch Set 1:

(2 comments)

https://gerrit.asterisk.org/c/asterisk/+/15763/1/res/res_pjsip_dialog_info_body_generator.c 
File res/res_pjsip_dialog_info_body_generator.c:

https://gerrit.asterisk.org/c/asterisk/+/15763/1/res/res_pjsip_dialog_info_body_generator.c@156 
PS1, Line 156: 		from_domain = endpoint ? (!ast_strlen_zero(endpoint->fromdomain) ? endpoint->fromdomain : invalid) : NULL;
> The concern here was that 'endpoint' info may not contain 'fromdomain' info, so in order to prevent  […]
No, I'm saying that from_domain is being set to endpoint->fromdomain without holding a reference to endpoint (to guarantee that endpoint remains valid, and thus endpoint->fromdomain remains valid). This is fine, however, because the subscription holds a reference to endpoint. That's why I stated about adding a comment so if someone sees this they'll understand it's fine.


https://gerrit.asterisk.org/c/asterisk/+/15763/1/res/res_pjsip_dialog_info_body_generator.c@207 
PS1, Line 207: 			need = strlen(connected_num) + (connected_num_restricted ? strlen(invalid) :
> I didn't write this line, I presume it's to prevent injection and buffer overflow. […]
if need exceeds PJSIP_MAX_URL_SIZE for some reason then this would actually cause a buffer overflow, as snprintf accepts the maximum size to write. It should instead use sizeof(remote_target) I believe.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15763
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I20c5cf5b45f34d7179df6573c5abf863eb72964b
Gerrit-Change-Number: 15763
Gerrit-PatchSet: 1
Gerrit-Owner: Joe <ynadiv at corpit.xyz>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-CC: Friendly Automation
Gerrit-Comment-Date: Wed, 14 Apr 2021 17:37:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Joshua Colp <jcolp at sangoma.com>
Comment-In-Reply-To: Joe <ynadiv at corpit.xyz>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210414/6c2ec07e/attachment.html>