| Message ID | 20241122134410.124563-1-mngyadam@amazon.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | cifs: Fix buffer overflow when parsing NFS reparse points | expand |
On Friday 22 November 2024 14:44:10 Mahmoud Adam wrote: > From: Pali Rohár <pali@kernel.org> > > upstream e2a8910af01653c1c268984855629d71fb81f404 commit. > > ReparseDataLength is sum of the InodeType size and DataBuffer size. > So to get DataBuffer size it is needed to subtract InodeType's size from > ReparseDataLength. > > Function cifs_strndup_from_utf16() is currentlly accessing buf->DataBuffer > at position after the end of the buffer because it does not subtract > InodeType size from the length. Fix this problem and correctly subtract > variable len. > > Member InodeType is present only when reparse buffer is large enough. Check > for ReparseDataLength before accessing InodeType to prevent another invalid > memory access. > > Major and minor rdev values are present also only when reparse buffer is > large enough. Check for reparse buffer size before calling reparse_mkdev(). > > Fixes: d5ecebc4900d ("smb3: Allow query of symlinks stored as reparse points") > Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.com> > Signed-off-by: Pali Rohár <pali@kernel.org> > Signed-off-by: Steve French <stfrench@microsoft.com> > [use variable name symlink_buf, the other buf->InodeType accesses are > not used in current version so skip] > Signed-off-by: Mahmoud Adam <mngyadam@amazon.com> > --- > This fixes CVE-2024-49996, and applies cleanly on 5.4->6.1, 6.6 and > later already has the fix. Interesting... I have not know that there is CVE number for this issue. Have you asked for assigning CVE number? Or was it there before? > fs/smb/client/smb2ops.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c > index d1e5ff9a3cd39..fcfbc096924a8 100644 > --- a/fs/smb/client/smb2ops.c > +++ b/fs/smb/client/smb2ops.c > @@ -2897,6 +2897,12 @@ parse_reparse_posix(struct reparse_posix_data *symlink_buf, > > /* See MS-FSCC 2.1.2.6 for the 'NFS' style reparse tags */ > len = le16_to_cpu(symlink_buf->ReparseDataLength); > + if (len < sizeof(symlink_buf->InodeType)) { > + cifs_dbg(VFS, "srv returned malformed nfs buffer\n"); > + return -EIO; > + } > + > + len -= sizeof(symlink_buf->InodeType); > > if (le64_to_cpu(symlink_buf->InodeType) != NFS_SPECFILE_LNK) { > cifs_dbg(VFS, "%lld not a supported symlink type\n", > -- > 2.40.1 >
Pali Rohár <pali@kernel.org> writes: > On Friday 22 November 2024 14:44:10 Mahmoud Adam wrote: >> From: Pali Rohár <pali@kernel.org> >> >> upstream e2a8910af01653c1c268984855629d71fb81f404 commit. >> >> ReparseDataLength is sum of the InodeType size and DataBuffer size. >> So to get DataBuffer size it is needed to subtract InodeType's size from >> ReparseDataLength. >> >> Function cifs_strndup_from_utf16() is currentlly accessing buf->DataBuffer >> at position after the end of the buffer because it does not subtract >> InodeType size from the length. Fix this problem and correctly subtract >> variable len. >> >> Member InodeType is present only when reparse buffer is large enough. Check >> for ReparseDataLength before accessing InodeType to prevent another invalid >> memory access. >> >> Major and minor rdev values are present also only when reparse buffer is >> large enough. Check for reparse buffer size before calling reparse_mkdev(). >> >> Fixes: d5ecebc4900d ("smb3: Allow query of symlinks stored as reparse points") >> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.com> >> Signed-off-by: Pali Rohár <pali@kernel.org> >> Signed-off-by: Steve French <stfrench@microsoft.com> >> [use variable name symlink_buf, the other buf->InodeType accesses are >> not used in current version so skip] >> Signed-off-by: Mahmoud Adam <mngyadam@amazon.com> >> --- >> This fixes CVE-2024-49996, and applies cleanly on 5.4->6.1, 6.6 and >> later already has the fix. > > Interesting... I have not know that there is CVE number for this issue. > Have you asked for assigning CVE number? Or was it there before? > Nope, It was assigned a CVE here: https://lore.kernel.org/all/2024102138-CVE-2024-49996-0d29@gregkh/ -MNAdam
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index d1e5ff9a3cd39..fcfbc096924a8 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -2897,6 +2897,12 @@ parse_reparse_posix(struct reparse_posix_data *symlink_buf, /* See MS-FSCC 2.1.2.6 for the 'NFS' style reparse tags */ len = le16_to_cpu(symlink_buf->ReparseDataLength); + if (len < sizeof(symlink_buf->InodeType)) { + cifs_dbg(VFS, "srv returned malformed nfs buffer\n"); + return -EIO; + } + + len -= sizeof(symlink_buf->InodeType); if (le64_to_cpu(symlink_buf->InodeType) != NFS_SPECFILE_LNK) { cifs_dbg(VFS, "%lld not a supported symlink type\n",