And to be frank- framing my input as me slagging on Yann is grotesque. You ship GA releases as a team, and so when you ship a dud like 2.17 you should take your lumps as a team.
Again, you know how to put processes in place to ensure adequate peer review is happening, just like you know whimsical patches like the one at fault here do not belong in a grave security release that 2.17 was slated to be.
At this point it?s water under the bridge. Release 2.18, when you see fit, and if it?s not insane for us to put it on CPAN, we will.
Get Outlook for iOS<
https://aka.ms/o0ukef>
________________________________
From: Joe Schaefer <joe@sunstarsys.com>
Sent: Sunday, October 30, 2022 12:09:02 AM
To: dev@httpd.apache.org <dev@httpd.apache.org>
Subject: Re: [libapreq2] nits to pick about the patches to util.c over the past few years
Forgive me for summarizing, but I didn?t come here expecting help, much less collaboration on a solution. I came here expecting to be scolded for having the temerity to critique the quality of the patch sets you?ve been shipping of late In libapreq2. None of my opinions have changed on that subject, and won?t for some time.
The point is I?m part of your extended support channels for libapreq2, because it?s easy and fun to help people who use the code. I?m not here to complain, I?m here expecting more empathy for people like me who give their time graciously without expecting anything in return other than some measure of respect for the effort involved.
On Sat, Oct 29, 2022 at 9:59 PM Joe Schaefer <joe@sunstarsys.com<mailto:joe@sunstarsys.com>> wrote:
Missed one. The patch that introduced these changes was revision=1895107.
On Sat, Oct 29, 2022 at 9:15 PM Joe Schaefer <joe@sunstarsys.com<mailto:joe@sunstarsys.com>> wrote:
Of course, I don?t know how to advise you regarding the security aspects, since you?re doing what you thought was the right thing to do and put the mfd parser into an error state instead of leaving well enough alone. But basically libapreq2 users get annoyed when the parser breaks on valid input, and may get antsy when their server goes bonkers because they aren?t in the habit of doing error handling on this condition.
On Sat, Oct 29, 2022 at 8:36 PM Joe Schaefer <joe@sunstarsys.com<mailto:joe@sunstarsys.com>> wrote:
Found the problem: it's just a misunderstanding about what is admissible in a successful file upload widget.
If someone doesn't add a file to the upload widget, it is still a successful control and should be processed as such on the server.
In this case, just like with opera, the filename attribute will be present, but set to an empty double-quoted string.
Here's my patchset, enjoy.
On Sat, Oct 29, 2022 at 2:47 PM Joe Schaefer <joe@sunstarsys.com<mailto:joe@sunstarsys.com>> wrote:
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 <joe@sunstarsys.com<mailto:joe@sunstarsys.com>> wrote:
On Sat, Oct 29, 2022 at 1:16 PM Joe Schaefer <joe@sunstarsys.com<mailto:joe@sunstarsys.com>> wrote:
On Sat, Oct 29, 2022 at 1:00 PM Yann Ylavic <ylavic.dev@gmail.com<mailto:ylavic.dev@gmail.com>> wrote:
Hi Joe,
here comes the "goofer".
On Fri, Oct 28, 2022 at 9:05 PM <joe@sunstarsys.com<mailto:joe@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.
[
https://ci3.googleusercontent.com/mail-sig/AIorK4xJ9wGYA7VWN-zW0DcpKll4IC6JxLGTMmDkmdn4h4eHliQhOGGu1nAHJcSkYVnw1jXF8E--UGA]
We only build what you need built.
<joe@sunstarsys.com<mailto:joe@sunstarsys.com>>
954.253.3732<tel://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.
[
https://ci3.googleusercontent.com/mail-sig/AIorK4xJ9wGYA7VWN-zW0DcpKll4IC6JxLGTMmDkmdn4h4eHliQhOGGu1nAHJcSkYVnw1jXF8E--UGA]
We only build what you need built.
<joe@sunstarsys.com<mailto:joe@sunstarsys.com>>
954.253.3732<tel://954.253.3732>
--
Joe Schaefer, Ph.D.
[
https://ci3.googleusercontent.com/mail-sig/AIorK4xJ9wGYA7VWN-zW0DcpKll4IC6JxLGTMmDkmdn4h4eHliQhOGGu1nAHJcSkYVnw1jXF8E--UGA]
We only build what you need built.
<joe@sunstarsys.com<mailto:joe@sunstarsys.com>>
954.253.3732<tel://954.253.3732>
--
Joe Schaefer, Ph.D.
[
https://ci3.googleusercontent.com/mail-sig/AIorK4xJ9wGYA7VWN-zW0DcpKll4IC6JxLGTMmDkmdn4h4eHliQhOGGu1nAHJcSkYVnw1jXF8E--UGA]
We only build what you need built.
<joe@sunstarsys.com<mailto:joe@sunstarsys.com>>
954.253.3732<tel://954.253.3732>
--
Joe Schaefer, Ph.D.
[
https://ci3.googleusercontent.com/mail-sig/AIorK4xJ9wGYA7VWN-zW0DcpKll4IC6JxLGTMmDkmdn4h4eHliQhOGGu1nAHJcSkYVnw1jXF8E--UGA]
We only build what you need built.
<joe@sunstarsys.com<mailto:joe@sunstarsys.com>>
954.253.3732<tel://954.253.3732>
--
Joe Schaefer, Ph.D.
[
https://ci3.googleusercontent.com/mail-sig/AIorK4xJ9wGYA7VWN-zW0DcpKll4IC6JxLGTMmDkmdn4h4eHliQhOGGu1nAHJcSkYVnw1jXF8E--UGA]
We only build what you need built.
<joe@sunstarsys.com<mailto:joe@sunstarsys.com>>
954.253.3732<tel://954.253.3732>
--
Joe Schaefer, Ph.D.
[
https://ci3.googleusercontent.com/mail-sig/AIorK4xJ9wGYA7VWN-zW0DcpKll4IC6JxLGTMmDkmdn4h4eHliQhOGGu1nAHJcSkYVnw1jXF8E--UGA]
We only build what you need built.
<joe@sunstarsys.com<mailto:joe@sunstarsys.com>>
954.253.3732<tel://954.253.3732>
openpgp-digital-signature.asc (3.82 KB)
