Re: [libapreq2] nits to pick about the patches to util.c over the past few years

[Joe Schaefer]

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

From: Joe Schaefer 
Sent: Sunday, October 30, 2022 12:09:02 AM
To: 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 
mailto:j...@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 
mailto:j...@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 
mailto:j...@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 
mailto:j...@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 
mailto:j...@sunstarsys.com>> wrote:

On Sat, Oct 29, 2022 at 1:16 PM Joe Schaefer 
mailto:j...@sunstarsys.com>> wrote:


On Sat, Oct 29, 2022 at 1:00 PM Yann Ylavic 
mailto:ylavic@gmail.com>> wrote:
Hi Joe,

here comes the "goofer".

On Fri, Oct 28, 2022 at 9:05 PM 
mailto: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, 
> 

Bug report for Apache httpd-2 [2022/10/30]

[bugzilla]

+---+
| Bugzilla Bug ID   |
| +-+
| | Status: UNC=Unconfirmed NEW=New ASS=Assigned|
| | OPN=ReopenedVER=Verified(Skipped Closed/Resolved)   |
| |   +-+
| |   | Severity: BLK=Blocker CRI=Critical  REG=Regression  MAJ=Major   |
| |   |   MIN=Minor   NOR=NormalENH=Enhancement TRV=Trivial |
| |   |   +-+
| |   |   | Date Posted |
| |   |   |  +--+
| |   |   |  | Description  |
| |   |   |  |  |
|10747|New|Maj|2002-07-12|ftp SIZE command and 'smart' ftp servers results i|
|11580|Opn|Enh|2002-08-09|generate Content-Location headers |
|12033|Opn|Nor|2002-08-26|Graceful restart immediately result in [warn] long|
|13661|Ass|Enh|2002-10-15|Apache cannot not handle dynamic IP reallocation  |
|14104|Opn|Enh|2002-10-30|not documented: must restart server to load new CR|
|16811|Ass|Maj|2003-02-05|mod_autoindex always return webpages in UTF-8.|
|17244|Ass|Nor|2003-02-20|./configure --help gives false information regardi|
|17497|Opn|Nor|2003-02-27|mod_mime_magic generates incorrect response header|
|20036|Ass|Nor|2003-05-19|Trailing Dots stripped from PATH_INFO environment |
|21260|Opn|Nor|2003-07-02|CacheMaxExpire directive not enforced !   |
|21533|Ass|Cri|2003-07-11|Multiple levels of htacces files can cause mod_aut|
|22484|Opn|Maj|2003-08-16|semaphore problem takes httpd down|
|22686|Opn|Nor|2003-08-25|ab: apr_poll: The timeout specified has expired (7|
|22898|Opn|Nor|2003-09-02|nph scripts with two HTTP header  |
|23911|Opn|Cri|2003-10-18|CGI processes left defunct/zombie under 2.0.54|
|24095|Opn|Cri|2003-10-24|ERROR "Parent: child process exited with status 32|
|24437|Opn|Nor|2003-11-05|mod_auth_ldap doubly-escapes backslash (\) charact|
|24890|Opn|Nor|2003-11-21|Apache config parser should not be local aware ( g|
|25469|Opn|Enh|2003-12-12|create AuthRoot for defining paths to auth files  |
|25484|Ass|Nor|2003-12-12|Non-service Apache cannot be stopped in WinXP |
|26153|Opn|Cri|2004-01-15|Apache cygwin directory traversal vulnerability   |
|27257|Ass|Enh|2004-02-26|rotatelogs with getopt and setuid |
|27715|Ass|Enh|2004-03-16|Client sending misformed Range "bytes = 0-100" ins|
|29090|Ass|Enh|2004-05-19|MultiviewsMatch NegotiatedOnly extensions not resp|
|29510|Ass|Enh|2004-06-10|ab does not support multiple cookies  |
|29644|Ver|Nor|2004-06-17|mod_proxy keeps downloading even after the client |
|30259|Ass|Enh|2004-07-22|When proxy connects to backend, a DNS lookup is do|
|30505|Ass|Enh|2004-08-05|Apache uses 'Error', and not lower level event typ|
|31302|Opn|Cri|2004-09-19|suexec doesn't execute commands if they're not in |
|31352|Ass|Enh|2004-09-21|RFE, Bind to LDAP server with browser supplier use|
|31418|Opn|Nor|2004-09-25|SSLUserName is not usable by other modules|
|32328|Opn|Enh|2004-11-19|Make mod_rewrite escaping optional / expose intern|
|32750|Ass|Maj|2004-12-17|mod_proxy + Win32DisableAcceptEx = memory leak|
|33089|New|Nor|2005-01-13|mod_include: Options +Includes (or IncludesNoExec)|
|34519|New|Enh|2005-04-19|Directory index should emit valid XHTML   |
|35098|Ver|Maj|2005-05-27|Install fails using --prefix  |
|35154|Opn|Nor|2005-06-01|Support for NID_serialNumber, etc. in SSLUserName |
|35652|Opn|Min|2005-07-07|Improve error message: "pcfg_openfile: unable to c|
|35768|Opn|Nor|2005-07-17|Missing file logs at far too high of log level|
|36636|Opn|Maj|2005-09-13|database write lock taken for PROPFIND operations |
|36676|New|Nor|2005-09-15|time() bug in httpd/os/win32/util_win32.c:wait_for|
|36710|Opn|Blk|2005-09-19|CGI output not captured   |
|37006|Ver|Reg|2005-10-11|"pthread" error when compiling under AIX 5.3 using|
|37290|Opn|Min|2005-10-28|DirectoryIndex don't work in scriptaliased directo|
|37564|New|Enh|2005-11-19|Suggestion: mod_suexec SuexecUserGroup directive i|
|38325|Opn|Nor|2006-01-20|impossible to determine AUTH_TYPE of interpreted r|
|38571|New|Enh|2006-02-08|CustomLog directive checked by apachectl configtes|
|38995|New|Nor|2006-03-16|httpd tries to communicate with the CGI daemon eve|
|39275|Opn|Nor|2006-04-11|slow child_init causes MaxClients warning |
|39287|New|Nor|2006-04-12|Incorrect If-Modified-Since validation (due to syn|
|39727|Ass|Nor|2006-06-05|Incorrect ETag on gzip:ed content |