Libguestfs - Aug 2014 - Re: [PATCH] p2v: check results of strndup and sscanf

---
 p2v/ssh.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/p2v/ssh.c b/p2v/ssh.c
index 1e9b05c..ff906df 100644
--- a/p2v/ssh.c
+++ b/p2v/ssh.c
@@ -505,7 +505,16 @@ open_data_connection (struct config *config, int
*local_port, int *remote_port)
                        }, ovector, ovecsize)) {
   case 100:                     /* Ephemeral port. */
     port_str = strndup (&h->buffer[ovector[2]], ovector[3]-ovector[2]);
-    sscanf (port_str, "%d", remote_port);
+    if (port_str == NULL) {
+      set_ssh_error ("not enough memory for strndup");
+      mexp_close (h);
+      return NULL;
+    }
+    if (sscanf (port_str, "%d", remote_port) != 1) {
+      set_ssh_error ("cannot extract the port number from
'%s'", port_str);
+      mexp_close (h);
+      return NULL;
+    }
     break;
 
   case MEXP_EOF:
-- 
1.9.3

On Mon, Aug 11, 2014 at 06:35:50PM +0200, Pino Toscano
wrote:
> ---
>  p2v/ssh.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/p2v/ssh.c b/p2v/ssh.c
> index 1e9b05c..ff906df 100644
> --- a/p2v/ssh.c
> +++ b/p2v/ssh.c
> @@ -505,7 +505,16 @@ open_data_connection (struct config *config, int
*local_port, int *remote_port)
>                         }, ovector, ovecsize)) {
>    case 100:                     /* Ephemeral port. */
>      port_str = strndup (&h->buffer[ovector[2]],
ovector[3]-ovector[2]);
> -    sscanf (port_str, "%d", remote_port);
> +    if (port_str == NULL) {
> +      set_ssh_error ("not enough memory for strndup");
> +      mexp_close (h);
> +      return NULL;
> +    }
> +    if (sscanf (port_str, "%d", remote_port) != 1) {
> +      set_ssh_error ("cannot extract the port number from
'%s'", port_str);
> +      mexp_close (h);
> +      return NULL;
> +    }
>      break;
ACK, although the regex ought to guarantee this already?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On Tuesday 12 August 2014 10:54:08 Richard W.M. Jones
wrote:
> On Mon, Aug 11, 2014 at 06:35:50PM +0200, Pino Toscano wrote:
> > ---
> > 
> >  p2v/ssh.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/p2v/ssh.c b/p2v/ssh.c
> > index 1e9b05c..ff906df 100644
> > --- a/p2v/ssh.c
> > +++ b/p2v/ssh.c
> > @@ -505,7 +505,16 @@ open_data_connection (struct config *config,
> > int *local_port, int *remote_port)> 
> >                         }, ovector, ovecsize)) {
> >    
> >    case 100:                     /* Ephemeral port. */
> >    
> >      port_str = strndup (&h->buffer[ovector[2]],
> >      ovector[3]-ovector[2]);
> > 
> > -    sscanf (port_str, "%d", remote_port);
> > +    if (port_str == NULL) {
> > +      set_ssh_error ("not enough memory for strndup");
> > +      mexp_close (h);
> > +      return NULL;
> > +    }
> > +    if (sscanf (port_str, "%d", remote_port) != 1) {
> > +      set_ssh_error ("cannot extract the port number from
'%s'",
> > port_str); +      mexp_close (h);
> > +      return NULL;
> > +    }
> > 
> >      break;
> 
> ACK, although the regex ought to guarantee this already?
Yes, it does -- this is just a small safety net for sscanf.

Thanks,
-- 
Pino Toscano