Hello,
On Wed, Feb 06, 2019 at 02:28:27PM -0200, Ricardo Nabinger Sanchez wrote:
> Hello,
>
> scan-build found a 28-step path where an unitialized value could be used in
> h2s_htx_bck_make_req_headers().
>
> Here is a shortened version:
>
> 4378 idx = htx_get_head(htx); // returns the SL that we skip
> 4379 while ((idx = htx_get_next(htx, idx)) != -1) {
> 4380 blk = htx_get_blk(htx, idx);
> 4381 type = htx_get_blk_type(blk);
> 4382
> 4383 if (type == HTX_BLK_UNUSED)
> 4384 continue;
> 4385
> 4386 if (type != HTX_BLK_HDR)
> // (here, assume condition is true, so control leaves the loop...)
> 4387 break;
> 4388
> 4389 if (unlikely(hdr >= sizeof(list)/sizeof(list[0]) - 1))
> 4390 goto fail;
> 4391
> // (... and list will not be initialized.)
> 4392 list[hdr].n = htx_get_blk_name(htx, blk);
> 4393 list[hdr].v = htx_get_blk_value(htx, blk);
> 4394 hdr++;
> 4395 }
>
> ...
The most important piece was skipped here :
/* marker for end of headers */
list[hdr].n = ist("");
So the list is *always* terminated.
> 4450 /* look for the Host header and place it in :authority */
> 4451 auth = ist2(NULL, 0);
> 4452 for (hdr = 0; hdr < sizeof(list)/sizeof(list[0]); hdr++)
> {
> 4453 if (isteq(list[hdr].n, ist("")))
> // (here, assume the condition is false, so control keeps in this block...)
> 4454 break; // end
Here the only way to reach this point is to have met an initialized
element, since the first one after the last initialized one sets the
name to the empty string and causes the break to happen.
> 4455
> 4456 if (isteq(list[hdr].n, ist("host"))) {
> 4457 auth = list[hdr].v;
> // (... auth receives an uninitialized value from list ...)
Thus no by definition.
> 4458 break;
> 4459 }
> 4460 }
> 4461 }
(...)
> While this feels like a convoluted or unlikely scenario, the path leading
> to the use of uninitialized value seems to be correctly unearthed by
> scan-build.
Fortunately no, it's a classical false positive instead.
Thanks!
Willy
