Any concerns on this? @Ruediger Pluem ? On Wed, Jul 3, 2024 at 9:29 PM Eric Covener <cove...@gmail.com> wrote: > > On Tue, Jul 2, 2024 at 7:34 AM Evgeny Kotkov via dev > <dev@httpd.apache.org> wrote: > > > > Hi all, > > > > Currently, `httpd.h` redefines the `apr_filepath_merge()` function on Win32 > > as follows: > > ``` > > #ifdef WIN32 > > #define apr_filepath_merge ap_filepath_merge > > #endif > > ``` > > > > The intent behind this redefinition seems to be to retroactively apply > > the recently introduced UNC path checks to any existing calls of > > apr_filepath_merge(), including those in third-party module code. > > > > However, this approach has several downsides, because the security-related > > behavior of the code is now tied to the inclusion of <httpd.h>: > > > > - Utility layers that do not include httpd.h but use apr_filepath_merge() > > will need to include this header to ensure appropriate security behavior. > > > > - Refactorings that involve adding or removing #include <httpd.h> can > > inadvertently change the security properties of the code. > > > > - It becomes challenging to understand the code behavior by inspection > > alone. > > > > Personally, I find these issues quite significant. > > > > An alternative would be to explicitly use ap_filepath_merge() everywhere and > > remove the platform-specific define. This alternative is demonstrated in > > the > > attached patch: > > [[[ > > * include/httpd.h > > (apr_filepath_merge): Remove this Win32-specific #define, and … > > > > * modules/*, server/*: > > …replace all its usages with ap_filepath_merge(). This ensures explicit > > and > > consistent security-related behavior that doesn't depend on the inclusion > > of the <httpd.h> header. > > ]]] > > > > What do you think? > > It certainly has a weird look/smell as-is but security@ was on the fence. > I think there was probably a bias towards staying where we started, > but I think it's reasonable to swap it out especially if anyone has > strong feelings about it as above.
-- Eric Covener cove...@gmail.com
