[1/2] smsutil: Use a safer strlcpy

Commit Message

Denis Kenzior Feb. 29, 2024, 11:56 p.m. UTC

sms_address_from_string is meant as private API, to be used with string
form addresses that have already been sanitized.  However, to be safe,
use a safe version of strcpy to avoid overflowing the buffer in case the
input was not sanitized properly.  While here, add a '__' prefix to the
function name to help make it clearer that this API is private and
should be used with more care.
---
 src/smsutil.c   | 14 +++++++-------
 src/smsutil.h   |  2 +-
 unit/test-sms.c |  6 +++---
 3 files changed, 11 insertions(+), 11 deletions(-)

Comments

[e-mail] March 1, 2024, 5:20 p.m. UTC | #1

Hello:

This series was applied to ofono.git (master)
by Denis Kenzior <[e-mail]>:

On Thu, 29 Feb 2024 17:56:41 -0600 you wrote:
> sms_address_from_string is meant as private API, to be used with string
> form addresses that have already been sanitized.  However, to be safe,
> use a safe version of strcpy to avoid overflowing the buffer in case the
> input was not sanitized properly.  While here, add a '__' prefix to the
> function name to help make it clearer that this API is private and
> should be used with more care.
> 
> [...]

Here is the summary with links:
  - [1/2] smsutil: Use a safer strlcpy
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=8fa1fdfcb54e
  - [2/2] voicecall: Refactor string_to_phone_number
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=1ff99673d693

You are awesome, thank you!

Patch

diff --git a/src/smsutil.c b/src/smsutil.c
index 6fe2bcb4477f..5934da9055dd 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -1887,15 +1887,15 @@  time_t sms_scts_to_time(const struct sms_scts *scts, struct tm *remote)
 	return ret;
 }
 
-void sms_address_from_string(struct sms_address *addr, const char *str)
+void __sms_address_from_string(struct sms_address *addr, const char *str)
 {
 	addr->numbering_plan = SMS_NUMBERING_PLAN_ISDN;
 	if (str[0] == '+') {
 		addr->number_type = SMS_NUMBER_TYPE_INTERNATIONAL;
-		strcpy(addr->address, str + 1);
+		l_strlcpy(addr->address, str + 1, sizeof(addr->address));
 	} else {
 		addr->number_type = SMS_NUMBER_TYPE_UNKNOWN;
-		strcpy(addr->address, str);
+		l_strlcpy(addr->address, str, sizeof(addr->address));
 	}
 }
 
@@ -3084,7 +3084,7 @@  gboolean status_report_assembly_report(struct status_report_assembly *assembly,
 		}
 	}
 
-	sms_address_from_string(&addr, straddr);
+	__sms_address_from_string(&addr, straddr);
 
 	if (pending == TRUE && node->deliverable == TRUE) {
 		/*
@@ -3177,7 +3177,7 @@  void status_report_assembly_expire(struct status_report_assembly *assembly,
 	while (g_hash_table_iter_next(&iter_addr, (gpointer) &straddr,
 					(gpointer) &id_table)) {
 
-		sms_address_from_string(&addr, straddr);
+		__sms_address_from_string(&addr, straddr);
 		g_hash_table_iter_init(&iter_node, id_table);
 
 		/* Go through different messages. */
@@ -3470,7 +3470,7 @@  GSList *sms_datagram_prepare(const char *to,
 	template.submit.vp.relative = 0xA7; /* 24 Hours */
 	template.submit.dcs = 0x04; /* Class Unspecified, 8 Bit */
 	template.submit.udhi = TRUE;
-	sms_address_from_string(&template.submit.daddr, to);
+	__sms_address_from_string(&template.submit.daddr, to);
 
 	offset = 1;
 
@@ -3597,7 +3597,7 @@  GSList *sms_text_prepare_with_alphabet(const char *to, const char *utf8,
 	template.submit.srr = use_delivery_reports;
 	template.submit.mr = 0;
 	template.submit.vp.relative = 0xA7; /* 24 Hours */
-	sms_address_from_string(&template.submit.daddr, to);
+	__sms_address_from_string(&template.submit.daddr, to);
 
 	/* There are two enums for the same thing */
 	dialect = (enum gsm_dialect)alphabet;
diff --git a/src/smsutil.h b/src/smsutil.h
index 5389757c309c..98e10e15de72 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -465,7 +465,7 @@  int sms_udl_in_bytes(guint8 ud_len, guint8 dcs);
 time_t sms_scts_to_time(const struct sms_scts *scts, struct tm *remote);
 
 const char *sms_address_to_string(const struct sms_address *addr);
-void sms_address_from_string(struct sms_address *addr, const char *str);
+void __sms_address_from_string(struct sms_address *addr, const char *str);
 
 const guint8 *sms_extract_common(const struct sms *sms, gboolean *out_udhi,
 					guint8 *out_dcs, guint8 *out_udl,
diff --git a/unit/test-sms.c b/unit/test-sms.c
index 154bb33ed244..66755f34e603 100644
--- a/unit/test-sms.c
+++ b/unit/test-sms.c
@@ -1603,7 +1603,7 @@  static void test_sr_assembly(void)
 			sr3.status_report.mr);
 	}
 
-	sms_address_from_string(&addr, "[phone]");
+	__sms_address_from_string(&addr, "[phone]");
 
 	sra = status_report_assembly_new(NULL);
 
@@ -1626,7 +1626,7 @@  static void test_sr_assembly(void)
 	 * Send sms-message in the national address-format,
 	 * but receive in the international address-format.
 	 */
-	sms_address_from_string(&addr, "9911630");
+	__sms_address_from_string(&addr, "9911630");
 	status_report_assembly_add_fragment(sra, sha1, &addr, 4, time(NULL), 2);
 	status_report_assembly_add_fragment(sra, sha1, &addr, 5, time(NULL), 2);
 
@@ -1641,7 +1641,7 @@  static void test_sr_assembly(void)
 	 * Send sms-message in the international address-format,
 	 * but receive in the national address-format.
 	 */
-	sms_address_from_string(&addr, "[phone]");
+	__sms_address_from_string(&addr, "[phone]");
 	status_report_assembly_add_fragment(sra, sha1, &addr, 6, time(NULL), 1);
 
 	g_assert(status_report_assembly_report(sra, &sr3, id, &delivered));