Re: unitialized buffer used in error situation

[Mark D. Baushke]

Derek Robert Price <derek@ximbiot.com> writes:

> Mark D. Baushke wrote:
> 
> |Todd C. Miller <Todd.Miller@courtesan.com> writes:
> |
> |>One of the OpenBSD developers (David Krause) recently ran into a
> |>cvs crash caused by the use of an unitialized buffer.  I examined
> |>the traceback and found the source of the crash.  The simple fix
> |>follows.
> 
> Mark, I think we're better off than before after this patch, but it you
> look where make_file_label is called in src/diff.c and the result then
> passed to diff, it looks like the label can shift to the wrong file when
> the first call to make_file_label returns a NULL:

Yes, the code in diff_exec() does appear to assume that label1 will not
be NULL if label2 is not NULL.

> 
> ~    call_diff_setup (args);
> ~    if (label1)
> ~        call_diff_arg (label1);
> ~    if (label2)
> ~        call_diff_arg (label2);
> ~    call_diff_arg ("--");
> ~    call_diff_arg (file1);
> ~    call_diff_arg (file2);
> ~    free (args);
> 
> If label1 is NULL, then diff interprets the first label argument, in
> this case label2, as being attached to file1.

A missing label2 is not as much of a problem as that will be filled in
by the file2 argument that is sent later. If both label1 and label2 are
NULL, then it is not a problem. If label1 is not NULL and label2 is NULL,
then that is not a problem.

Does the following code make sense?

    if (label1 && label2)
    {
        call_diff_arg (label1);
        call_diff_arg (label2);
    }
    else if (label1)
    {
        call_diff_arg (label1);
    }
    else if (label2)
    {
        call_diff_arg ("-L<unknown-file>\t<unknown-date>");
        call_diff_arg (label2);
    }

> I'm not sure what could cause the call to CVS_STAT to fail and then the
> label not to be set, but I think the correct fix here is either to make
> the failed stat a fatal error or to create a label with just PATH or the
> like.  Todd, do you know what was causing the CVS_STAT command to fail
> on your reporter's system?

This is a good question.

        -- Mark