ntfs3: fix out-of-bounds read in ntfs_dir_emit() and hdr_find_e()

[PATCH] ntfs3: fix out-of-bounds read in ntfs_dir_emit() and hdr_find_e()

Posted by Alessandro Schino 1 month ago

The bounds check in ntfs_dir_emit() compares fname->name_len (a
character count) against e->size (a byte count) without accounting
for the 2-byte-per-character UTF-16LE encoding or the ATTR_FILE_NAME
header size:

  if (fname->name_len + sizeof(struct NTFS_DE) > le16_to_cpu(e->size))

This computes: name_len + 16 > e_size

The correct check must account for the ATTR_FILE_NAME header (66 bytes
before the name) and the UTF-16LE character size (2 bytes each):

  sizeof(NTFS_DE) + offsetof(ATTR_FILE_NAME, name) +
  name_len * sizeof(short) > e_size

Which computes: 16 + 66 + name_len * 2 > e_size

The correct calculation already exists as fname_full_size() in ntfs.h
and is used in cmp_fnames(), namei.c, and fslog.c, but was not used
in the readdir path.

A crafted NTFS image with an index entry containing a small e->size
but large fname->name_len bypasses the current check, causing
ntfs_utf16_to_nls() to read past the entry boundary.

Additionally, add a key_size validation in hdr_find_e() to ensure the
declared key_size does not exceed the available entry data, preventing
comparison functions from reading past entry boundaries on the lookup
path.

Signed-off-by: Alessandro Schino <7991aleschino@gmail.com>
---
 fs/ntfs3/dir.c   | 4 +++-
 fs/ntfs3/index.c | 4 ++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
index d99ab086..6cd66ce0 100644
--- a/fs/ntfs3/dir.c
+++ b/fs/ntfs3/dir.c
@@ -305,7 +305,9 @@ static inline bool ntfs_dir_emit(struct ntfs_sb_info *sbi,
 	if (sbi->options->nohidden && (fname->dup.fa & FILE_ATTRIBUTE_HIDDEN))
 		return true;
 
-	if (fname->name_len + sizeof(struct NTFS_DE) > le16_to_cpu(e->size))
+	if (sizeof(struct NTFS_DE) +
+	    offsetof(struct ATTR_FILE_NAME, name) +
+	    fname->name_len * sizeof(short) > le16_to_cpu(e->size))
 		return true;
 
 	name_len = ntfs_utf16_to_nls(sbi, fname->name, fname->name_len, name,
diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
index 5344b29b..46dc7642 100644
--- a/fs/ntfs3/index.c
+++ b/fs/ntfs3/index.c
@@ -754,6 +754,10 @@ static struct NTFS_DE *hdr_find_e(const struct ntfs_index *indx,
 binary_search:
 	e_key_len = le16_to_cpu(e->key_size);
 
+	/* Validate key_size fits within the entry data area. */
+	if (e_key_len > le16_to_cpu(e->size) - sizeof(struct NTFS_DE))
+		return NULL;
+
 	diff2 = (*cmp)(key, key_len, e + 1, e_key_len, ctx);
 	if (diff2 > 0) {
 		if (found) {
-- 
2.43.0

Re: [PATCH] ntfs3: fix out-of-bounds read in ntfs_dir_emit() and hdr_find_e()

Posted by Konstantin Komarov 2 weeks, 1 day ago

On 5/11/26 20:15, Alessandro Schino wrote:

> [You don't often get email from 7991aleschino@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> The bounds check in ntfs_dir_emit() compares fname->name_len (a
> character count) against e->size (a byte count) without accounting
> for the 2-byte-per-character UTF-16LE encoding or the ATTR_FILE_NAME
> header size:
>
>    if (fname->name_len + sizeof(struct NTFS_DE) > le16_to_cpu(e->size))
>
> This computes: name_len + 16 > e_size
>
> The correct check must account for the ATTR_FILE_NAME header (66 bytes
> before the name) and the UTF-16LE character size (2 bytes each):
>
>    sizeof(NTFS_DE) + offsetof(ATTR_FILE_NAME, name) +
>    name_len * sizeof(short) > e_size
>
> Which computes: 16 + 66 + name_len * 2 > e_size
>
> The correct calculation already exists as fname_full_size() in ntfs.h
> and is used in cmp_fnames(), namei.c, and fslog.c, but was not used
> in the readdir path.
>
> A crafted NTFS image with an index entry containing a small e->size
> but large fname->name_len bypasses the current check, causing
> ntfs_utf16_to_nls() to read past the entry boundary.
>
> Additionally, add a key_size validation in hdr_find_e() to ensure the
> declared key_size does not exceed the available entry data, preventing
> comparison functions from reading past entry boundaries on the lookup
> path.
>
> Signed-off-by: Alessandro Schino <7991aleschino@gmail.com>
> ---
>   fs/ntfs3/dir.c   | 4 +++-
>   fs/ntfs3/index.c | 4 ++++
>   2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
> index d99ab086..6cd66ce0 100644
> --- a/fs/ntfs3/dir.c
> +++ b/fs/ntfs3/dir.c
> @@ -305,7 +305,9 @@ static inline bool ntfs_dir_emit(struct ntfs_sb_info *sbi,
>          if (sbi->options->nohidden && (fname->dup.fa & FILE_ATTRIBUTE_HIDDEN))
>                  return true;
>
> -       if (fname->name_len + sizeof(struct NTFS_DE) > le16_to_cpu(e->size))
> +       if (sizeof(struct NTFS_DE) +
> +           offsetof(struct ATTR_FILE_NAME, name) +
> +           fname->name_len * sizeof(short) > le16_to_cpu(e->size))
>                  return true;
>
>          name_len = ntfs_utf16_to_nls(sbi, fname->name, fname->name_len, name,
> diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
> index 5344b29b..46dc7642 100644
> --- a/fs/ntfs3/index.c
> +++ b/fs/ntfs3/index.c
> @@ -754,6 +754,10 @@ static struct NTFS_DE *hdr_find_e(const struct ntfs_index *indx,
>   binary_search:
>          e_key_len = le16_to_cpu(e->key_size);
>
> +       /* Validate key_size fits within the entry data area. */
> +       if (e_key_len > le16_to_cpu(e->size) - sizeof(struct NTFS_DE))
> +               return NULL;
> +
>          diff2 = (*cmp)(key, key_len, e + 1, e_key_len, ctx);
>          if (diff2 > 0) {
>                  if (found) {
> --
> 2.43.0
>
Hello,

Your patch was applied, thank you.

Regards,
Konstantin