Re: file upload name filtering

And the same for 7.x
https://github.com/apache/struts/pull/1219

niedz., 16 lut 2025 o 08:07 Lukasz Lenart <[email protected]> napisał(a):
>
> PR reverts changes in 6.7.x
> https://github.com/apache/struts/pull/1218
>
> sob., 15 lut 2025 o 08:10 Kusal Kithul-Godage
> <[email protected]> napisał(a):
> >
> > Ah that's right - the MultiPartRequestWrapper augments the regular
> > request parameters with multipart form fields.
> >
> > Yes I think in that case it's sensible to revert the new validation
> > changes in AbstractMultiPartRequest (and subclasses) and get the other
> > patches released first.
> >
> > And then we can devise a new solution that only targets filenames. A
> > sanitizing solution sounds like a good idea. If implemented as a
> > protected method on AbstractMultiPartRequest, applications can easily
> > disable or customise the behaviour using the existing
> > 'struts.multipart.parser' extension point.
> >
> > On Fri, Feb 14, 2025 at 5:16 AM Lukasz Lenart <[email protected]> wrote:
> > >
> > > Right now I'm looking at the "defaultStack":
> > >
> > > ....
> > > <interceptor-ref name="actionFileUpload"/>
> > > ...
> > > <interceptor-ref name="params"/>
> > > ...
> > >
> > > and escaping/normalising normal form fields doesn't make sense as this
> > > will happen in "params" interceptor. What should happen is a better
> > > control over file names.
> > >
> > > czw., 13 lut 2025 o 19:00 Lukasz Lenart <[email protected]> napisał(a):
> > > >
> > > > My idea for that was related to what we are doing in
> > > > ParametersInterceptor, but looks like this requires a step back and
> > > > rethink. I would revert these changes for now if there are no
> > > > objections.
> > > >
> > > > czw., 13 lut 2025 o 09:27 Greg Huber <[email protected]> napisał(a):
> > > > >
> > > > > Unlikely a sanitizing method might not work for everybody.  I have had
> > > > > quite a few attempts to sanitize the way I want (first I run the
> > > > > Normalizer which removes all sorts of unwanted characters, then I only
> > > > > allow alphanumeric and then truncate the length) so to make one that
> > > > > fits all will not be easy.
> > > > >
> > > > > We do not want to be rejecting file uploads with no way of overriding it.
> > > > >
> > > > > The longer the file name the higher the risk?
> > > > >
> > > > > On 12/02/2025 20:04, Burton Rhodes wrote:
> > > > > > If that’s the case, do you think a sanitize method is more appropriate for this situation? Perhaps one that could live inside an interceptor and possibly even be overridden by those who think they know what they’re doing?  This way you wouldn’t have to reject the upload which in my opinion is not a preferred user experience.
> > > > > >
> > > > > >
> > > > > >> On Feb 12, 2025, at 12:56 PM, Brian Andle<[email protected]> wrote:
> > > > > >>
> > > > > >> Unless I'm mistaken this is to prevent issues when a developer uses the
> > > > > >> file name, unsanitized, and potentially other malicious type injection via
> > > > > >> specially crafted file names.
> > > > > >>
> > > > > >>> On Wed, Feb 12, 2025, 10:05 AM Burton Rhodes<[email protected]> wrote:
> > > > > >>>
> > > > > >>> I agree with Greg.
> > > > > >>>
> > > > > >>> IMHO, character validation should be left to the developer which depends
> > > > > >>> on their OS and file names supported therein. But if there needs to be
> > > > > >>> protection against a buffer overflow attack (I assume that is the
> > > > > >>> problem you are trying to solve?), then the length restriction should
> > > > > >>> suffice.  Or is there another risk I'm not aware of that could threaten
> > > > > >>> a system by just having a few malicious characters in a file name?
> > > > > >>>
> > > > > >>>
> > > > > >>> Thanks,
> > > > > >>> Burton
> > > > > >>>
> > > > > >>>
> > > > > >>> ------ Original Message ------
> > > > > >>>  From "Greg Huber"<[email protected]>
> > > > > >>> [email protected]
> > > > > >>> Date 2/11/2025 2:51:36 AM
> > > > > >>> Subject Re: file upload name filtering
> > > > > >>>
> > > > > >>>> Filename length is a possible good way to go, with an override of the
> > > > > >>> length and then truncate or block option.
> > > > > >>>> On 11/02/2025 06:21, Lukasz Lenart wrote:
> > > > > >>>>> Hm... looks like I must re-think this approach, thanks all for
> > > > > >>>>> reporting this issue!
> > > > > >>> ---------------------------------------------------------------------
> > > > > >>> To unsubscribe, e-mail:[email protected]
> > > > > >>> For additional commands, e-mail:[email protected]
> > > > > >>>
> > > > > >>>
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [email protected]
> > > For additional commands, e-mail: [email protected]
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [email protected]
> > For additional commands, e-mail: [email protected]
> >