[Asterisk-code-review] various modules: json integer overflow (...asterisk[13])

George Joseph asteriskteam at digium.com
Tue Aug 6 11:07:08 CDT 2019


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

Change subject: various modules: json integer overflow
......................................................................

various modules: json integer overflow

There were still a few places in the code that could overflow when "packing"
a json object with a value outside the base type integer's range. For instance:

unsigned int value = INT_MAX + 1
ast_json_pack("{s: i}", value);

would result in a negative number being "packed". In those situations this patch
alters those values to a ast_json_int_t, which widens the value up to a long or
long long.

ASTERISK-28480

Change-Id: Ied530780d83e6f1772adba0e28d8938ef30c49a1
---
M apps/app_agent_pool.c
M apps/app_queue.c
M channels/chan_iax2.c
M funcs/func_talkdetect.c
M main/aoc.c
M main/ccss.c
M main/channel.c
M main/core_local.c
M main/rtp_engine.c
M main/stasis_channels.c
10 files changed, 38 insertions(+), 38 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/apps/app_agent_pool.c b/apps/app_agent_pool.c
index 07040f6..b2068c1 100644
--- a/apps/app_agent_pool.c
+++ b/apps/app_agent_pool.c
@@ -1459,9 +1459,9 @@
 
 	ast_assert(agent != NULL);
 
-	blob = ast_json_pack("{s: s, s: i}",
+	blob = ast_json_pack("{s: s, s: I}",
 		"agent", agent,
-		"logintime", logintime);
+		"logintime", (ast_json_int_t)logintime);
 	if (!blob) {
 		return;
 	}
diff --git a/apps/app_queue.c b/apps/app_queue.c
index c5508e3..53ae2ba 100644
--- a/apps/app_queue.c
+++ b/apps/app_queue.c
@@ -5679,12 +5679,12 @@
 		break;
 	}
 
-	blob = ast_json_pack("{s: s, s: s, s: s, s: i, s: i, s: s}",
+	blob = ast_json_pack("{s: s, s: s, s: s, s: I, s: I, s: s}",
 		"Queue", queuename,
 		"Interface", member->interface,
 		"MemberName", member->membername,
-		"HoldTime", (long)(callstart - holdstart),
-		"TalkTime", (long)(time(NULL) - callstart),
+		"HoldTime", (ast_json_int_t)(callstart - holdstart),
+		"TalkTime", (ast_json_int_t)(time(NULL) - callstart),
 		"Reason", reason ?: "");
 
 	queue_publish_multi_channel_snapshot_blob(ast_queue_topic(queuename), caller, peer,
@@ -6955,12 +6955,12 @@
 		ast_queue_log(queuename, ast_channel_uniqueid(qe->chan), member->membername, "CONNECT", "%ld|%s|%ld", (long) (time(NULL) - qe->start), ast_channel_uniqueid(peer),
 													(long)(orig - to > 0 ? (orig - to) / 1000 : 0));
 
-		blob = ast_json_pack("{s: s, s: s, s: s, s: i, s: i}",
+		blob = ast_json_pack("{s: s, s: s, s: s, s: I, s: I}",
 				     "Queue", queuename,
 				     "Interface", member->interface,
 				     "MemberName", member->membername,
-				     "HoldTime", (long) (time(NULL) - qe->start),
-				     "RingTime", (long)(orig - to > 0 ? (orig - to) / 1000 : 0));
+				     "HoldTime", (ast_json_int_t)(time(NULL) - qe->start),
+				     "RingTime", (ast_json_int_t)(orig - to > 0 ? (orig - to) / 1000 : 0));
 		queue_publish_multi_channel_blob(qe->chan, peer, queue_agent_connect_type(), blob);
 
 		ast_copy_string(oldcontext, ast_channel_context(qe->chan), sizeof(oldcontext));
diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index 43f7d2c..3bf06d0 100644
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -11137,18 +11137,18 @@
 						if (iaxs[fr->callno]->pingtime <= peer->maxms) {
 							ast_log(LOG_NOTICE, "Peer '%s' is now REACHABLE! Time: %u\n", peer->name, iaxs[fr->callno]->pingtime);
 							ast_endpoint_set_state(peer->endpoint, AST_ENDPOINT_ONLINE);
-							blob = ast_json_pack("{s: s, s: i}",
+							blob = ast_json_pack("{s: s, s: I}",
 								"peer_status", "Reachable",
-								"time", iaxs[fr->callno]->pingtime);
+								"time", (ast_json_int_t)iaxs[fr->callno]->pingtime);
 							ast_devstate_changed(AST_DEVICE_NOT_INUSE, AST_DEVSTATE_CACHABLE, "IAX2/%s", peer->name); /* Activate notification */
 						}
 					} else if ((peer->historicms > 0) && (peer->historicms <= peer->maxms)) {
 						if (iaxs[fr->callno]->pingtime > peer->maxms) {
 							ast_log(LOG_NOTICE, "Peer '%s' is now TOO LAGGED (%u ms)!\n", peer->name, iaxs[fr->callno]->pingtime);
 							ast_endpoint_set_state(peer->endpoint, AST_ENDPOINT_ONLINE);
-							blob = ast_json_pack("{s: s, s: i}",
+							blob = ast_json_pack("{s: s, s: I}",
 								"peer_status", "Lagged",
-								"time", iaxs[fr->callno]->pingtime);
+								"time", (ast_json_int_t)iaxs[fr->callno]->pingtime);
 							ast_devstate_changed(AST_DEVICE_UNAVAILABLE, AST_DEVSTATE_CACHABLE, "IAX2/%s", peer->name); /* Activate notification */
 						}
 					}
diff --git a/funcs/func_talkdetect.c b/funcs/func_talkdetect.c
index 9700a24..3a7b2ad 100644
--- a/funcs/func_talkdetect.c
+++ b/funcs/func_talkdetect.c
@@ -205,7 +205,7 @@
 			int64_t diff_ms = ast_tvdiff_ms(ast_tvnow(), td_params->talking_start);
 			diff_ms -= td_params->dsp_silence_threshold;
 
-			blob = ast_json_pack("{s: i}", "duration", diff_ms);
+			blob = ast_json_pack("{s: I}", "duration", (ast_json_int_t)diff_ms);
 			if (!blob) {
 				return 1;
 			}
diff --git a/main/aoc.c b/main/aoc.c
index 725e910..c797610 100644
--- a/main/aoc.c
+++ b/main/aoc.c
@@ -1739,13 +1739,13 @@
 				decoded->aoc_s_entries[i].rate.duration.amount,
 				decoded->aoc_s_entries[i].rate.duration.multiplier);
 
-			time = ast_json_pack("{s:i, s:i}",
-				"Length", decoded->aoc_s_entries[i].rate.duration.time,
+			time = ast_json_pack("{s:I, s:i}",
+				"Length", (ast_json_int_t)decoded->aoc_s_entries[i].rate.duration.time,
 				"Scale", decoded->aoc_s_entries[i].rate.duration.time_scale);
 
 			if (decoded->aoc_s_entries[i].rate.duration.granularity_time) {
-				granularity = ast_json_pack("{s:i, s:i}",
-					"Length", decoded->aoc_s_entries[i].rate.duration.granularity_time,
+				granularity = ast_json_pack("{s:I, s:i}",
+					"Length", (ast_json_int_t)decoded->aoc_s_entries[i].rate.duration.granularity_time,
 					"Scale", decoded->aoc_s_entries[i].rate.duration.granularity_time_scale);
 			}
 
diff --git a/main/ccss.c b/main/ccss.c
index 205dc1b..b27287e 100644
--- a/main/ccss.c
+++ b/main/ccss.c
@@ -1082,9 +1082,9 @@
 {
 	struct ast_json *extras;
 
-	extras = ast_json_pack("{s: s, s: i}",
+	extras = ast_json_pack("{s: s, s: I}",
 		"caller", caller,
-		"expires", expires);
+		"expires", (ast_json_int_t)expires);
 
 	cc_publish(ast_cc_offertimerstart_type(), core_id, extras);
 	ast_json_unref(extras);
diff --git a/main/channel.c b/main/channel.c
index e95eac0..ff25c98 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -3688,10 +3688,10 @@
 	RAII_VAR(struct ast_json *, blob, NULL, ast_json_unref);
 	char digit_str[] = { digit, '\0' };
 
-	blob = ast_json_pack("{ s: s, s: s, s: i }",
+	blob = ast_json_pack("{ s: s, s: s, s: I }",
 		"digit", digit_str,
 		"direction", dtmf_direction_to_string(direction),
-		"duration_ms", duration_ms);
+		"duration_ms", (ast_json_int_t)duration_ms);
 	if (!blob) {
 		return;
 	}
diff --git a/main/core_local.c b/main/core_local.c
index 12e41f9..59c789a 100644
--- a/main/core_local.c
+++ b/main/core_local.c
@@ -416,8 +416,8 @@
 		return;
 	}
 
-	json_object = ast_json_pack("{s: i, s: i}",
-			"dest", dest, "id", id);
+	json_object = ast_json_pack("{s: i, s: I}",
+			"dest", dest, "id", (ast_json_int_t)id);
 
 	if (!json_object) {
 		return;
@@ -458,7 +458,7 @@
 		return;
 	}
 
-	json_object = ast_json_pack("{s: i, s: i}", "success", success, "id", id);
+	json_object = ast_json_pack("{s: i, s: I}", "success", success, "id", (ast_json_int_t)id);
 
 	if (!json_object) {
 		return;
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index e1a1e0b..c56ec5f 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -2535,14 +2535,14 @@
 		char str_lsr[32];
 
 		snprintf(str_lsr, sizeof(str_lsr), "%u", payload->report->report_block[i]->lsr);
-		json_report_block = ast_json_pack("{s: I, s: i, s: i, s: i, s: i, s: s, s: i}",
+		json_report_block = ast_json_pack("{s: I, s: I, s: I, s: I, s: I, s: s, s: I}",
 			"source_ssrc", (ast_json_int_t)payload->report->report_block[i]->source_ssrc,
-			"fraction_lost", payload->report->report_block[i]->lost_count.fraction,
-			"packets_lost", payload->report->report_block[i]->lost_count.packets,
-			"highest_seq_no", payload->report->report_block[i]->highest_seq_no,
-			"ia_jitter", payload->report->report_block[i]->ia_jitter,
+			"fraction_lost", (ast_json_int_t)payload->report->report_block[i]->lost_count.fraction,
+			"packets_lost", (ast_json_int_t)payload->report->report_block[i]->lost_count.packets,
+			"highest_seq_no", (ast_json_int_t)payload->report->report_block[i]->highest_seq_no,
+			"ia_jitter", (ast_json_int_t)payload->report->report_block[i]->ia_jitter,
 			"lsr", str_lsr,
-			"dlsr", payload->report->report_block[i]->dlsr);
+			"dlsr", (ast_json_int_t)payload->report->report_block[i]->dlsr);
 		if (!json_report_block
 			|| ast_json_array_append(json_rtcp_report_blocks, json_report_block)) {
 			ast_json_unref(json_rtcp_report_blocks);
@@ -2556,21 +2556,21 @@
 
 		snprintf(sec, sizeof(sec), "%lu", (unsigned long)payload->report->sender_information.ntp_timestamp.tv_sec);
 		snprintf(usec, sizeof(usec), "%lu", (unsigned long)payload->report->sender_information.ntp_timestamp.tv_usec);
-		json_rtcp_sender_info = ast_json_pack("{s: s, s: s, s: i, s: i, s: i}",
+		json_rtcp_sender_info = ast_json_pack("{s: s, s: s, s: I, s: I, s: I}",
 			"ntp_timestamp_sec", sec,
 			"ntp_timestamp_usec", usec,
-			"rtp_timestamp", payload->report->sender_information.rtp_timestamp,
-			"packets", payload->report->sender_information.packet_count,
-			"octets", payload->report->sender_information.octet_count);
+			"rtp_timestamp", (ast_json_int_t)payload->report->sender_information.rtp_timestamp,
+			"packets", (ast_json_int_t)payload->report->sender_information.packet_count,
+			"octets", (ast_json_int_t)payload->report->sender_information.octet_count);
 		if (!json_rtcp_sender_info) {
 			ast_json_unref(json_rtcp_report_blocks);
 			return NULL;
 		}
 	}
 
-	json_rtcp_report = ast_json_pack("{s: I, s: i, s: i, s: o, s: o}",
+	json_rtcp_report = ast_json_pack("{s: I, s: I, s: i, s: o, s: o}",
 		"ssrc", (ast_json_int_t)payload->report->ssrc,
-		"type", payload->report->type,
+		"type", (ast_json_int_t)payload->report->type,
 		"report_count", payload->report->reception_report_count,
 		"sender_information", json_rtcp_sender_info ?: ast_json_null(),
 		"report_blocks", json_rtcp_report_blocks);
diff --git a/main/stasis_channels.c b/main/stasis_channels.c
index cc7ab7d..58d52bf 100644
--- a/main/stasis_channels.c
+++ b/main/stasis_channels.c
@@ -1111,11 +1111,11 @@
 		return NULL;
 	}
 
-	return ast_json_pack("{s: s, s: o, s: s, s: i, s: o}",
+	return ast_json_pack("{s: s, s: o, s: s, s: I, s: o}",
 		"type", "ChannelDtmfReceived",
 		"timestamp", ast_json_timeval(*tv, NULL),
 		"digit", digit,
-		"duration_ms", duration_ms,
+		"duration_ms", (ast_json_int_t)duration_ms,
 		"channel", json_channel);
 }
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: Ied530780d83e6f1772adba0e28d8938ef30c49a1
Gerrit-Change-Number: 11667
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190806/04d981ea/attachment-0001.html>


More information about the asterisk-code-review mailing list