Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (athena.apache.org: domain of i.galic@brainsware.org
 designates 188.40.115.121 as permitted sender)
Date: Thu, 09 Jun 2011 21:39:07 -0000 (UTC)
From: Igor =?utf-8?Q?Gali=C4=87?= <i.galic@brainsware.org>
To: dev@httpd.apache.org
Subject: Re: Possible uninitialized variable in mod_data.c
Message-ID: <c89be4a5-9083-4123-9e5b-bc67998d307c@iris>
In-Reply-To: <20110609131419.GA30290@localhost>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0


----- Original Message -----
> Hi Folks,
> 
> We recently started using Sentry (static analysis tool) to analyze
> apache httpd on a nightly basis. Sentry found a potential
> unintialized
> variable in mod_data.c added in commit 1133582.


I think our human review machine already caught that one
http://mail-archives.apache.org/mod_mbox/httpd-dev/201106.mbox/%3C4DF07362.=
3060501@apache.org%3E

> I'm not sure if this case is actually possible at runtime, but
> I'll describe it here. Note, you can view the file I'm talking
> about here,
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_data.c=
?revision=3D1133582&view=3Dmarkup&pathrev=3D1133582
> 
> static apr_status_t data_out_filter(...
> {
>     ...
>     if (!ctx) {
>         ...
>         // EVENT 1: charset is uninitialized
>         char *charset;
>         ...
>         // EVENT 2: Take false path here
>         if (!ap_is_initial_req(f->r)) {
>             ap_remove_output_filter(f);
>             return ap_pass_brigade(f->next, bb);
>         }
>         ...
>         type =3D apr_pstrdup(r->pool, r->content_type);
>         // EVENT 3: take false path here
>         if (type) {
>             charset =3D strchr(type, ' ');
>             if (charset) {
>                 *charset++ =3D 0;
>                 end =3D strchr(charset, ' ');
>                 if (end) {
>                     *end++ =3D 0;
>                 }
>             }
>         }
> 
>         // EVENT 4: charset is used uninitialized.
>         // If it's possible to reach this case, you could potentially
>         // pass bogus data into the second %s.
>         apr_brigade_printf(ctx->bb, NULL, NULL, "data:%s%s;base64,",
>                 type ? type : "", charset ? charset : "");
> 
> If this case is reachable, I would suggest a patch like this:
> 
> - char *charset;
> + char *charset =3D 0;
>                                                                          =
                
> Thanks,
> Chris
> 
> --
> Chris Wilson
> http://vigilantsw.com/
> Vigilant Software, LLC
> 

-- 
Igor Gali=C4=87

Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/