Re: reading uninitialized buffer - Mailing list pgsql-patches

... and here it is. As for the test being outside the "if" statement, it
is true that that might waste a few cycles, but it hardly matters.
Personally, I would prefer to replace the if statement with this:

    if (c == EOF || c == '\n')
    {
        *buf = '\0';
        return;
    }

and then it wouldn't be an issue at all, but I know some people don't
like early function returns - is there a general postgres style rule
about it?

cheers

andrew


I wrote:

>
> This time it is my fault, rather than freebsd's ;-)
>
> I think I can do something slightly cleaner than this, though, by
> hoisting the buf termination above the test. We could also replace the
> strncmp calls with strcmp calls if the buffer has its nul. I will post
> something soon.
>
> cheers
>
> andrew
>
>
> Dennis Bjorklund wrote:
>
>> I've been testing pg using valgrind and have found a read of an
>> uninitialized buffer. In the hba-tokenizer when we have not read any
>> characters (or too few) we still perform a couple of:
>>
>>   strncmp(start_buf,"sameuser",8)
>>
>> Since this is done on random data it might return true although we have
>> not read anything. The result is that we can (even if the probability is
>> low) return the wrong thing.
>>
>> The solution is simply to terminate the buffer with '\0' before the
>> strncmp().
>>
>> I also moved our test inside the previous if, outside of that block our
>> test can never be true anyway. I don't know why it was outside in the
>> first place.
>>
>>
>>
>
Index: src/backend/libpq/hba.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/libpq/hba.c,v
retrieving revision 1.119
diff -c -w -r1.119 hba.c
*** src/backend/libpq/hba.c    25 Dec 2003 03:44:04 -0000    1.119
--- src/backend/libpq/hba.c    1 Feb 2004 13:53:51 -0000
***************
*** 169,187 ****
      }


      if ( !saw_quote &&
           (
!              strncmp(start_buf,"all",3) == 0  ||
!              strncmp(start_buf,"sameuser",8) == 0  ||
!              strncmp(start_buf,"samegroup",9) == 0
           )
          )
      {
          /* append newline to a magical keyword */
          *buf++ = '\n';
      }

-     *buf = '\0';

  }

--- 169,189 ----
      }


+     *buf = '\0';
+
      if ( !saw_quote &&
           (
!              strcmp(start_buf,"all") == 0  ||
!              strcmp(start_buf,"sameuser") == 0  ||
!              strcmp(start_buf,"samegroup") == 0
           )
          )
      {
          /* append newline to a magical keyword */
          *buf++ = '\n';
+         *buf = '\0';
      }


  }