[Asterisk-code-review] jansson-bundled: Patch for off-nominal crash. (asterisk[16])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/10609 )

Change subject: jansson-bundled: Patch for off-nominal crash.
......................................................................

jansson-bundled: Patch for off-nominal crash.

pack_string crashed on non-NULL strings returned when s->has_error was
true if the string was the result of 's' format without '#', '%' or '+'.

Change-Id: Ic125df691d81ba2cbc413e37bdae657b304d20d0
---
A third-party/jansson/patches/0035-Remove-inappropriate-jsonp_free-which-caused-segment.patch
1 file changed, 58 insertions(+), 0 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/third-party/jansson/patches/0035-Remove-inappropriate-jsonp_free-which-caused-segment.patch b/third-party/jansson/patches/0035-Remove-inappropriate-jsonp_free-which-caused-segment.patch
new file mode 100644
index 0000000..258fc67
--- /dev/null
+++ b/third-party/jansson/patches/0035-Remove-inappropriate-jsonp_free-which-caused-segment.patch
@@ -0,0 +1,58 @@
+From e262ea5fcd789d20d5d20d5d6d9c7ec06e3c00fd Mon Sep 17 00:00:00 2001
+From: Corey Farrell <git at cfware.com>
+Date: Mon, 5 Nov 2018 16:43:10 -0500
+Subject: [PATCH 35/35] Remove inappropriate jsonp_free which caused
+ segmentation fault.
+
+pack_string should never free str on error.  This wouldn't be a problem
+except the check for `ours` was inverted.  Just remove the check for
+ours since the true condition is unreachable.
+
+json_vpack_ex also had an error check for s.has_error.  This can never
+be true unless value is NULL.
+
+Test changes removed for merging into Asterisk bundled copy.
+
+Fixes #444
+---
+ src/pack_unpack.c           |  9 ++-------
+ test/suites/api/test_pack.c | 21 +++++++++++++++++++++
+ 2 files changed, 23 insertions(+), 7 deletions(-)
+
+diff --git a/src/pack_unpack.c b/src/pack_unpack.c
+index ec04bc3..3b99776 100644
+--- a/src/pack_unpack.c
++++ b/src/pack_unpack.c
+@@ -359,9 +359,7 @@ static json_t *pack_string(scanner_t *s, va_list *ap)
+         return t == '?' && !s->has_error ? json_null() : NULL;
+ 
+     if (s->has_error) {
+-        if (!ours)
+-            jsonp_free(str);
+-
++        /* It's impossible to reach this point if ours != 0, do not free str. */
+         return NULL;
+     }
+ 
+@@ -853,6 +851,7 @@ json_t *json_vpack_ex(json_error_t *error, size_t flags,
+     value = pack(&s, &ap_copy);
+     va_end(ap_copy);
+ 
++    /* This will cover all situations where s.has_error is true */
+     if(!value)
+         return NULL;
+ 
+@@ -862,10 +861,6 @@ json_t *json_vpack_ex(json_error_t *error, size_t flags,
+         set_error(&s, "<format>", json_error_invalid_format, "Garbage after format string");
+         return NULL;
+     }
+-    if(s.has_error) {
+-        json_decref(value);
+-        return NULL;
+-    }
+ 
+     return value;
+ }
+-- 
+2.17.2
+

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic125df691d81ba2cbc413e37bdae657b304d20d0
Gerrit-Change-Number: 10609
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181113/d7704fbb/attachment.html>