[perl #134008] unknown-crash in S_format_hexfp

On Wed, 01 May 2019 05:36:38 -0700, davem wrote:
> On Tue, Apr 16, 2019 at 01:58:32AM -0700, Hugo van der Sanden via RT
> wrote:
> > On Mon, 15 Apr 2019 18:17:44 -0700, tonyc wrote:
> > > On Tue, 09 Apr 2019 07:01:25 -0700, hv wrote:
> > > > On Tue, 09 Apr 2019 04:47:13 -0700, hv wrote:
> > > > > the tail code in sv_vcatpvfn_flags() after the comment:
> > > > >   /* append esignbuf, filler, zeros, eptr and dotstr to sv */
> > > >
> > > > It turns out the only use here is if 'zeros' is true, which is
> > > > only
> > > > set in a case where has_precis is true.
> > > >
> > > > So as far as I can tell, the only problem here is for hexfp, and
> > > > the
> > > > payload is to overwrite a number of bytes beyond the PL_efloatbuf
> > > > buffer with '0' characters, the number being controlled by a
> > > > sprintf
> > > > argument.
> > > >
> > > > As such I think the risk here is low - if a user can provide the
> > > > sprintf pattern, you've probably already lost, and likelihood of
> > > > a
> > > > '%.*a' pattern with user-supplied arguments seems remote. Nothing
> > > > in
> > > > the test suite uses negative precision except specific
> > > > sprintf{,2}
> > > > tests asserting it should be ignored (in the absence of which I'd
> > > > be
> > > > tempted to make it fatal instead).
> > > >
> > > > So I propose that we commit the attached belt-and-braces patch,
> > > > open
> > > > the ticket, and consider this for backporting. I'd welcome other
> > > > opinions.
> > >
> > > Somehow I didn't see Hugo's response (and his patch).
> > >
> > > An attacker doesn't need to supply a format string, they just need
> > > to
> > > be able to supply a negative precision, and it doesn't need to be
> > > large in magnitude.
> > >
> > > I could see a reporting tool allowing specifying width/precision
> > > for
> > > fields, though perhaps not so much supporting %a formatting.
> > >
> > > A case could be made that such a tool is buggy if it permits very
> > > large or negative precisions, but that's irrelevant as to whether a
> > > bug in perl becomes a security issue for such code.
> > >
> > > The behaviour for negative precision comes from the C standard:
> > >
> > > A negative precision argument is taken as if the precision were
> > > omitted.
> > >
> > > which presumably is what the current implementation is intended to
> > > emulate (especially since PerlIO_printf() uses this code.
> >
> > No problem with your analysis, I did request other opinions.
> >
> > I'd recommend adding the extra change in sv_vcatpvfn_flags from my
> > patch. Your test looks like the better one though.
> 
> I recommend we go with the hybrid as suggested by Hugo. I think we
> should
> add it to blead, and I don't think we need to treat it as a serious
> security issue. The string '%.*a' as part of a format appears nowhere
> on
> cpan.

Done in b0f5b1daacb21ab7e46a772a6ff0f70ca627cb58, 9dfe0a3438ae69872b71b98e4fb4f4bef084983d.

Tony

---
via perlbug:  queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=134008

• [perl #134008] unknown-crash in S_format_hexfp by Tony Cook via RT