On Tue, Sep 15, 2020 at 09:27:49AM -0400, Jeff Layton wrote:
> > > + }
> > > +
> > > + declen = fscrypt_base64_decode(name, len, tname->name);
> > > + if (declen < 0 || declen > NAME_MAX) {
> > > + ret = -EIO;
> > > + goto out;
> > > + }
> >
> > declen <= 0, to cover the empty name case.
> >
> > Also, is there a point in checking for > NAME_MAX?
> >
>
> IDK. We're getting these strings from the MDS and they could end up
> being corrupt if there are bugs there (or if the MDS is compromised).
> Of course, if we get a name longer than NAME_MAX then we've overrun the
> buffer.
>
> Maybe we should add a maxlen parameter to fscrypt_base64_encode/decode ?
> Or maybe I should just have fscrypt_fname_alloc_buffer allocate a buffer
> the same size as "len"? It might be a little larger than necessary, but
> that would be safer.
How about checking that the base64-encoded filename is <= BASE64_CHARS(NAME_MAX)
in length? Then decoding it can't give more than NAME_MAX bytes.
fscrypt_setup_filename() does a similar check when decoding a no-key name.
- Eric
|