[Libguestfs] [PATCH V3] virt-diff: add new virt-diff tool

[Wanlong Gao]

On 08/21/2012 04:25 PM, Richard W.M. Jones wrote:
> On Tue, Aug 21, 2012 at 04:04:54PM +0800, Wanlong Gao wrote:
> [...]
>>>> +  spid = fork ();
>>>> +  if (spid == -1) {
>>>> +    perror ("fork");
>>>> +    exit (EXIT_FAILURE);
>>>> +  }
>>>> +  if (spid == 0) {
>>>> +    if (guestfs_mount_local_run (sg) == -1)
>>>> +      exit (EXIT_FAILURE);
>>>> +    _exit (EXIT_SUCCESS);
>>>> +  }
>>>> +
>>>> +  dpid = fork();
>>>> +  if (dpid == -1) {
>>>> +    perror ("fork");
>>>> +    exit (EXIT_FAILURE);
>>>> +  }
>>>> +  if (dpid == 0) {
>>>> +    if (guestfs_mount_local_run (dg) == -1)
>>>> +      exit (EXIT_FAILURE);
>>>> +    _exit (EXIT_SUCCESS);
>>>> +  }
>>>> +
>>>> +  const char *dir = argv[optind];
>>>> +  char system_arg[BUFSIZ];
>>>> +  sprintf (system_arg, "diff -urN %s%s %s%s", stempdir, dir,
>>>> +                                              dtempdir, dir);
>>>> +  sleep (5);
>>>
>>> ?
>>
>> I think I'm waiting for guest launching?
> 
> No, that's the wrong thing to do.
> 
> The API is in two parts: First guestfs_mount_local prepares the mount
> point.  Second guestfs_mount_local_run causes requests to be
> processed.
> 
> You can access the mount point after guestfs_mount_local has returned.
> Your process will block until guestfs_mount_local_run is called, but
> that doesn't matter in this case (in fact, it's what you want).
> 
> Another thing: Don't use sprintf, ever.  With the wrong $TMPDIR that
> code above is a security hole.  Since you need to quote 'dir' (since
> it comes from user input) you're going to have to rewrite the whole
> call to system(3) so that it uses fork + exec instead.

Can you explain more? Sorry I can't understand where is the security
hole. And if I use fork + exec, how can I make "stempdir" and "dir"
together to an "argv[]" for the argument of exec?

Thanks,
Wanlong Gao

> 
> Rich.
>