iproute2: tc: potential buffer overflow

iproute2: tc: potential buffer overflow

[tomaspaukrt]

[-- Attachment #1: Type: text/plain, Size: 149 bytes --]

Hi,

there are two potentially dangerous calls of strcpy function in the program "tc". In the attachment is a patch that fixes this issue.

Tomas

[-- Attachment #2: iproute2-overflow-fix.patch --]
[-- Type: text/x-diff, Size: 945 bytes --]

diff --git a/tc/m_ipt.c b/tc/m_ipt.c
index cc95eab7..cb64380b 100644
--- a/tc/m_ipt.c
+++ b/tc/m_ipt.c
@@ -269,7 +269,8 @@ static int build_st(struct xtables_target *target, struct ipt_entry_target *t)
 		} else {
 			target->t = t;
 		}
-		strcpy(target->t->u.user.name, target->name);
+		strncpy(target->t->u.user.name, target->name,
+			sizeof(target->t->u.user.name) - 1);
 		return 0;
 	}
 
diff --git a/tc/m_xt_old.c b/tc/m_xt_old.c
index 6a4509a9..974ac496 100644
--- a/tc/m_xt_old.c
+++ b/tc/m_xt_old.c
@@ -177,7 +177,8 @@ build_st(struct xtables_target *target, struct xt_entry_target *t)
 	if (t == NULL) {
 		target->t = fw_calloc(1, size);
 		target->t->u.target_size = size;
-		strcpy(target->t->u.user.name, target->name);
+		strncpy(target->t->u.user.name, target->name,
+			sizeof(target->t->u.user.name) - 1);
 		set_revision(target->t->u.user.name, target->revision);
 
 		if (target->init != NULL)

Re: iproute2: tc: potential buffer overflow

[Stephen Hemminger]

On Sat, 31 Aug 2019 15:13:27 +0200 (CEST)
<tomaspaukrt@email.cz> wrote:

> Hi,
> 
> there are two potentially dangerous calls of strcpy function in the program "tc". In the attachment is a patch that fixes this issue.
> 
> Tomas

This looks correct.

Please fix with strlcpy() instead; that is clearer.
Plus you can use XT_EXTENSION_MAX_NAMELEN here (optional).

Re: iproute2: tc: potential buffer overflow

[tomaspaukrt]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

The updated patch is in the attachment.

---------- Původní e-mail ----------
Od: Stephen Hemminger <stephen@networkplumber.org>
Komu: tomaspaukrt@email.cz
Datum: 31. 8. 2019 17:38:01
Předmět: Re: iproute2: tc: potential buffer overflow
On Sat, 31 Aug 2019 15:13:27 +0200 (CEST)
<tomaspaukrt@email.cz> wrote:

> Hi,
>
> there are two potentially dangerous calls of strcpy function in the program "tc". In the attachment is a patch that fixes this issue.
>
> Tomas

This looks correct.

Please fix with strlcpy() instead; that is clearer.
Plus you can use XT_EXTENSION_MAX_NAMELEN here (optional).

[-- Attachment #2: iproute2-overflow-fix.patch --]
[-- Type: text/x-diff, Size: 1109 bytes --]

commit 46be35fbded24c75786ce178c516d7fba991a90a
Author: Tomas Paukrt <tomaspaukrt@email.cz>
Date:   Sat Sep 7 15:34:30 2019 +0200

    tc: fix potential buffer overflow

diff --git a/tc/m_ipt.c b/tc/m_ipt.c
index cc95eab..e47ae6b 100644
--- a/tc/m_ipt.c
+++ b/tc/m_ipt.c
@@ -269,7 +269,8 @@ static int build_st(struct xtables_target *target, struct ipt_entry_target *t)
 		} else {
 			target->t = t;
 		}
-		strcpy(target->t->u.user.name, target->name);
+		strlcpy(target->t->u.user.name, target->name,
+			sizeof(target->t->u.user.name));
 		return 0;
 	}
 
diff --git a/tc/m_xt_old.c b/tc/m_xt_old.c
index 6a4509a..dd27adf 100644
--- a/tc/m_xt_old.c
+++ b/tc/m_xt_old.c
@@ -177,7 +177,8 @@ build_st(struct xtables_target *target, struct xt_entry_target *t)
 	if (t == NULL) {
 		target->t = fw_calloc(1, size);
 		target->t->u.target_size = size;
-		strcpy(target->t->u.user.name, target->name);
+		strlcpy(target->t->u.user.name, target->name,
+			sizeof(target->t->u.user.name));
 		set_revision(target->t->u.user.name, target->revision);
 
 		if (target->init != NULL)