Curiously, this doesn't seem to present any problems for apreq_header_attribute in trunk/HEAD. A good thing.
That means we may need to look more closely at r1903484 in glue/perl. On Sat, Oct 29, 2022 at 2:12 PM Joe Schaefer <j...@sunstarsys.com> wrote: > > On Sat, Oct 29, 2022 at 1:16 PM Joe Schaefer <j...@sunstarsys.com> wrote: > >> >> >> On Sat, Oct 29, 2022 at 1:00 PM Yann Ylavic <ylavic....@gmail.com> wrote: >> >>> Hi Joe, >>> >>> here comes the "goofer". >>> >>> On Fri, Oct 28, 2022 at 9:05 PM <j...@sunstarsys.com> wrote: >>> > >>> > Long time fan, not a first time caller. >>> >>> Yet what a crappy thread (and comments on [1]). >>> All top posting, unreplyable. >>> >>> > >>> > Libapreq2 was intended to be a safe,fast, standards compliant library- >>> primarily *safe* before all other priorities. Some of the work going on >>> lately in util.c is starting to undermine that prime directive, so I’d like >>> to better understand why these changes are happening, and why they are >>> snowballing into a less functional, less secure software product that is >>> driving up my support costs on CPAN. >>> >>> Yeah sure, rewriting history. That marvelous previous 2.16 just >>> exploded when faced with google's oss-fuzzers (and not just a little, >>> quite some reports) which now fuzz httpd trunk (thus apreq). >>> CVE-2022-22728 is about libapreq2 v2.16 *and earlier" right? So >>> something pre-dated my changes. >> >> >> Fair enough. >> >> >>> >> > >>> > For instance, this revision 1867789 is a pure pessimization: it >>> trades userland RAM for filesystem cache RAM, that’s it, but it’s not a big >>> deal. Just churn. >>> >>> I call it a fix for an UAF (Use After Free). This is my only change in >>> 2.16 btw, while you seem to suggest that security issues started with >>> 2.16. >>> >>> > >>> > Everything in the crufty, old apreq_header_attribute code I wrote was >>> completely tossed and reimplemented. Why? >>> >>> Someone had to address the security reports, and someone (me) dared >>> touching your code because it was not safe (i.e. >>> broken/crashing/vulnerable/..), not for the lulz nor breaking users. >>> I'm very sorry if that happened, only those who do nothing do not >>> break anything though. >>> Existing tests were still passing, but shit happens. >>> >> >> Then lets deal with it by adding more tests. >> >> >>> >>> > We’re just racking up CVE’s, people are disabling the mfd parser >>> altogether, and it no longer support common use cases that people now >>> complain about because it supported cases in the wild that the new work >>> does not. >>> >>> Are there multiple issues? I know of the one reported in [1] about >>> "file upload does not work if any file fields are blank". >>> That's not actionable sorry (I don't understand what it means), no >>> more than your rant here and elusive "hints" on where/how to fix it. >>> I asked in the other thread for a reproducer in the form of a HTTP >>> payload, not a mod_perl handler which I don't know how to debug (let >>> alone without the right thing to send on the client side). >>> >>> >> I translated the bug report for you. It involves browsers like Opera >> that send filename="" >> attributes in the Content-Disposition header. It's generating an >> accidental DoS, depending >> on how people use the upload API. Toss in some tests into util.t and >> I'll add this one for you. >> >> >>> > >>> > With the latest code coming out of p5p for Perl, there’s a whole new >>> reason for excitement in httpd-land: the mod_perl2 + mpm_event combination >>> is rock solid and screaming fast with HTTP/2. The only reason I resubbed >>> here is in the hopes of some synergy retaking these perl-related projects, >>> since mod_perl2 is the only game in town for embedded interpreters in >>> httpd2 (and no, lua is not the answer, it’s not thread safe either). >>> >>> Synergy! What a great intro.. >>> >>> >>> Regards; >>> Yann. >>> >>> [1] https://rt.cpan.org/Public/Bug/Display.html?id=144470 >>> >> >> >> -- >> Joe Schaefer, Ph.D. >> We only build what you need built. >> <j...@sunstarsys.com> >> 954.253.3732 <//954.253.3732> >> >> >> > > Let's start with this (untested) patch... > > > Index: library/t/util.c > =================================================================== > --- library/t/util.c (revision 1904922) > +++ library/t/util.c (working copy) > @@ -271,6 +271,7 @@ > static void test_header_attribute(dAT, void *ctx) > { > const char hdr[] = "name=\"filename=foo\"; filename=\"quux.txt\""; > + const char opera[] = "name=\"foo\"; filename=\"\""; > const char *val; > apr_size_t vlen; > > @@ -284,6 +285,10 @@ > AT_int_eq(vlen, 8); > AT_mem_eq("quux.txt", val, 8); > > + AT_int_eq(apreq_header_attribute(opera, "filename" 8, &val, &vlen), > + APR_SUCCESS); > + AT_int_eq(vlen,0); > + > } > > static void test_brigade_concat(dAT, void *ctx) > @@ -315,7 +320,7 @@ > { dT(test_join, 0) }, > { dT(test_brigade_fwrite, 0) }, > { dT(test_file_mktemp, 0) }, > - { dT(test_header_attribute, 6) }, > + { dT(test_header_attribute, 8) }, > { dT(test_brigade_concat, 0) }, > }; > -- > Joe Schaefer, Ph.D. > We only build what you need built. > <j...@sunstarsys.com> > 954.253.3732 <//954.253.3732> > > > -- Joe Schaefer, Ph.D. We only build what you need built. <j...@sunstarsys.com> 954.253.3732 <//954.253.3732>
