[PATCH] Fix various uninitalized variable values. See CWE-457 for info.

[NIIBE Yutaka]

Thanks for your report.

Patch could be applied.  But, I'm afraid that it only makes enough
sense to shut up some static analyzer.

On 01/24/2015 12:41 AM, Joshua Rogers wrote:
> diff --git a/common/iobuf.c b/common/iobuf.c
> index badbf78..3b13483 100644
> --- a/common/iobuf.c
> +++ b/common/iobuf.c
> @@ -1476,7 +1476,7 @@ iobuf_openrw (const char *fname)
>    iobuf_t a;
>    gnupg_fd_t fp;
>    file_filter_ctx_t *fcx;
> -  size_t len;
> +  size_t len = 0;
>  
>    if (!fname)
>      return NULL;

It agree to apply this part of your patch for cleanness.

Since the call of 'file_filter' with IOBUFCTRL_DESC or IOBUFCTRL_INIT
doesn't touch its size, uninitialized access has no impact in fact.

> diff --git a/g10/textfilter.c b/g10/textfilter.c
> index 394d9c3..c6c4eec 100644
> --- a/g10/textfilter.c
> +++ b/g10/textfilter.c
> @@ -165,7 +165,7 @@ copy_clearsig_text( IOBUF out, IOBUF inp, gcry_md_hd_t md,
>  {
>      unsigned int maxlen;
>      byte *buffer = NULL;    /* malloced buffer */
> -    unsigned int bufsize;   /* and size of this buffer */
> +    unsigned int bufsize = 0;   /* and size of this buffer */
>      unsigned int n;
>      int truncated = 0;
>      int pending_lf = 0;

Umm...  IIUC, it is intended usage.

The third argument of iobuf_read_line is output-only when the second
argument is NULL.  Semantics of this API would be somehow complicated,
but it's useful for the loop.

It would be OK to initialize bufsize to 0 (or any value), though.

> diff --git a/sm/keydb.c b/sm/keydb.c
> index 974625d..7bbbbec 100644
> --- a/sm/keydb.c
> +++ b/sm/keydb.c
> @@ -958,7 +958,7 @@ int
>  keydb_search (KEYDB_HANDLE hd, KEYDB_SEARCH_DESC *desc, size_t ndesc)
>  {
>    int rc = -1;
> -  unsigned long skipped;
> +  unsigned long skipped = 0;
>  
>    if (!hd)
>      return gpg_error (GPG_ERR_INV_VALUE);

I agree to apply this part of your patch for correctness, even though
the value is not used.  It would be more cleaner API to allow NULL to
be supplied if the value is no interest.

> Please note: There are 3 more:
> /g10/keyring.c:
> 1011      byte afp[MAX_FINGERPRINT_LEN];

This is the output buffer for fingerprint.  With closer look of the
code, you can see it will always have valid fingerprint when accessed.
Initializing doesn't make sense.

> /g10/keygen.c:
> 305    byte sym[MAX_PREFS], hash[MAX_PREFS], zip[MAX_PREFS];

These are the output buffer for preferences.  The buffer SYM will be
used with NSYM, as NSYM limits boundary.  So, initializing NSYM only
is enough.  (Same for HASH and ZIP.)  Initializing the buffer itself
doesn't make sense.

> /g10/keylist.c:
> 770          char buf[(MAX_FINGERPRINT_LEN * 2) + 90];

This is the output buffer for sprintf output.  Initializing doesn't
make sense.
--