On Wed, Jun 22, 2016 at 12:16:27PM +0530, Anupam Kapoor wrote:

quoted

      if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
-             strncpy(pt->rx_dev, value, strlen(value));
+             strncpy(pt->rx_dev, value, sizeof(pt->rx_dev)-1);

I guess size-1 is to give room for terminating null byte, but for this
case is it guarantied that pt->rx_dev last byte is NULL?

why not just use a snprintf(...) here since it has better error behavior ?
although compared to str*cpy it might be a bit slow, but hopefully that
should be ok ?

Definite +1. For safely copying strings I think snprintf is often the easiest
API to use.

/Bruce

--
thanks
anupam


On Tue, Jun 21, 2016 at 10:51 PM, Ferruh Yigit [off-list ref]
wrote:

quoted

On 6/21/2016 4:18 PM, Reshma Pattan wrote:

quoted

using source length in strncpy can cause destination
overflow if destination length is not big enough to
handle the source string. Changes are made to use destination
size instead of source length in strncpy.

Coverity issue 127351: string overflow

Fixes: caa7028276b8 ("app/pdump: add tool for packet capturing")

Signed-off-by: Reshma Pattan <redacted>
---
 app/pdump/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/pdump/main.c b/app/pdump/main.c
index f8923b9..af92ef3 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c

@@ -217,12 +217,12 @@ parse_rxtxdev(const char *key, const char *value,

void *extra_args)

quoted

      struct pdump_tuples *pt = extra_args;

      if (!strcmp(key, PDUMP_RX_DEV_ARG)) {
-             strncpy(pt->rx_dev, value, strlen(value));
+             strncpy(pt->rx_dev, value, sizeof(pt->rx_dev)-1);

I guess size-1 is to give room for terminating null byte, but for this
case is it guarantied that pt->rx_dev last byte is NULL?

-- 
In the beginning was the lambda, and the lambda was with Emacs, and Emacs
was the lambda.