[ofa-general] Re: [PATCH] ibutils/ibis: prevent buffer overflows

[Yevgeny Kliteynik]

Hi Sasha,

Sasha Khapyorsky wrote:
> There are couple of one byte buffer overflows in ibis*_wrap.c* files.
> Guess those files where generated originally, but I didn't find from
> where stuff like obj->log_file[1024] = '\0' is coming. So fising in
> place.

Yeah, it wasn't so simple to find where do they come from.
description[IB_NODE_DESCRIPTION_SIZE] was relatively easy,
but the other one was tricky...
I'll send a v2 of your patch with the files that have the
origin of these bugs.

-- Yevgeny

> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> ---
>  ibis/src/ibis_wrap.c     |    4 ++--
>  ibis/src/ibissh_wrap.cpp |    4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/ibis/src/ibis_wrap.c b/ibis/src/ibis_wrap.c
> index 70bc3b2..85e72d5 100644
> --- a/ibis/src/ibis_wrap.c
> +++ b/ibis/src/ibis_wrap.c
> @@ -44884,7 +44884,7 @@ static int TclsmVlArbTableCmd(ClientData clientData, Tcl_Interp *interp, int obj
>  static ibsm_node_desc_str_t * _ibsm_node_desc_description_set(smNodeDesc *obj, ibsm_node_desc_str_t val[IB_NODE_DESCRIPTION_SIZE]) {
>  {
>    strncpy((char *)obj->description,(char *)val,IB_NODE_DESCRIPTION_SIZE - 1);
> -  obj->description[IB_NODE_DESCRIPTION_SIZE] = '\0';
> +  obj->description[IB_NODE_DESCRIPTION_SIZE - 1] = '\0';
>  }
>      return (ibsm_node_desc_str_t *) val;
>  }
> @@ -72782,7 +72782,7 @@ static int _wrap_ibis_opt_t_log_flags_get(ClientData clientData, Tcl_Interp *int
>  static char * _ibis_opt_log_file_set(ibis_opt_t *obj, char val[1024]) {
>  {
>    strncpy(obj->log_file,val,1024 - 1);
> -  obj->log_file[1024] = '\0';
> +  obj->log_file[1023] = '\0';
>  }
>      return (char *) val;
>  }
> diff --git a/ibis/src/ibissh_wrap.cpp b/ibis/src/ibissh_wrap.cpp
> index a794cc4..ece7c9c 100644
> --- a/ibis/src/ibissh_wrap.cpp
> +++ b/ibis/src/ibissh_wrap.cpp
> @@ -44946,7 +44946,7 @@ static int TclsmVlArbTableCmd(ClientData clientData, Tcl_Interp *interp, int obj
>  static ibsm_node_desc_str_t * _ibsm_node_desc_description_set(smNodeDesc *obj, ibsm_node_desc_str_t val[IB_NODE_DESCRIPTION_SIZE]) {
>  {
>    strncpy((char *)obj->description,(char *)val,IB_NODE_DESCRIPTION_SIZE - 1);
> -  obj->description[IB_NODE_DESCRIPTION_SIZE] = '\0';
> +  obj->description[IB_NODE_DESCRIPTION_SIZE - 1] = '\0';
>  }
>      return (ibsm_node_desc_str_t *) val;
>  }
> @@ -72844,7 +72844,7 @@ static int _wrap_ibis_opt_t_log_flags_get(ClientData clientData, Tcl_Interp *int
>  static char * _ibis_opt_log_file_set(ibis_opt_t *obj, char val[1024]) {
>  {
>    strncpy(obj->log_file,val,1024 - 1);
> -  obj->log_file[1024] = '\0';
> +  obj->log_file[1023] = '\0';
>  }
>      return (char *) val;
>  }