diff mbox series

lib: fix strcat with equivalent logic

Message ID 1550136631-32415-1-git-send-email-tallurix.chaitanya.babu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series lib: fix strcat with equivalent logic |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Chaitanya Babu, TalluriX Feb. 14, 2019, 9:30 a.m. UTC 
  Replace strcat with concatenation logic to avoid buffer overflow.

Fixes: a6a47ac9c2 ("cfgfile: rework load function")
Cc: stable@dpdk.org

Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
---
 lib/librte_cfgfile/rte_cfgfile.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Bruce Richardson Feb. 14, 2019, 2 p.m. UTC | #1 
On Thu, Feb 14, 2019 at 09:30:31AM +0000, Chaitanya Babu Talluri wrote:
> Replace strcat with concatenation logic to avoid buffer overflow.
> 
> Fixes: a6a47ac9c2 ("cfgfile: rework load function")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaitanya Babu Talluri <tallurix.chaitanya.babu@intel.com>
> ---
>  lib/librte_cfgfile/rte_cfgfile.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
> index 7d8c941ea..7616742f7 100644
> --- a/lib/librte_cfgfile/rte_cfgfile.c
> +++ b/lib/librte_cfgfile/rte_cfgfile.c
> @@ -227,7 +227,15 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
>  			while (end != NULL) {
>  				if (*(end+1) == params->comment_character) {
>  					*end = '\0';
> -					strcat(split[1], end+1);
> +					int index = strlen(split[1]);
> +					char *temp = end+1;
> +					while (*temp != '\0') {
> +						split[1][index] = *temp;
> +						index++;
> +						temp++;
> +					}
> +					split[1][index] = '\0';
> +

I don't see how this change fixes the problem. From what I see, you have
just inlined the strcat function into the code above, without changing any
of the logic. To prevent buffer overflows using strcat, you need to
identify the total buffer length and use that when copying. I suggest
reworking this code to use strlcat.

/Bruce
diff mbox series

Patch

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 7d8c941ea..7616742f7 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -227,7 +227,15 @@  rte_cfgfile_load_with_params(const char *filename, int flags,
 			while (end != NULL) {
 				if (*(end+1) == params->comment_character) {
 					*end = '\0';
-					strcat(split[1], end+1);
+					int index = strlen(split[1]);
+					char *temp = end+1;
+					while (*temp != '\0') {
+						split[1][index] = *temp;
+						index++;
+						temp++;
+					}
+					split[1][index] = '\0';
+
 				} else
 					end++;
 				end = memchr(end, '\\', strlen(end));