diff mbox

[CVE-2016-4913,precise,trusty,lts-utopic,vivid,wily] get_rock_ridge_filename(): handle malformed NM entries

Message ID 1464189758-27400-1-git-send-email-luis.henriques@canonical.com
State New
Headers show

Commit Message

Luis Henriques May 25, 2016, 3:22 p.m. UTC
From: Al Viro <viro@zeniv.linux.org.uk>

Payloads of NM entries are not supposed to contain NUL.  When we run
into such, only the part prior to the first NUL goes into the
concatenation (i.e. the directory entry name being encoded by a bunch
of NM entries).  We do stop when the amount collected so far + the
claimed amount in the current NM entry exceed 254.  So far, so good,
but what we return as the total length is the sum of *claimed*
sizes, not the actual amount collected.  And that can grow pretty
large - not unlimited, since you'd need to put CE entries in
between to be able to get more than the maximum that could be
contained in one isofs directory entry / continuation chunk and
we are stop once we'd encountered 32 CEs, but you can get about 8Kb
easily.  And that's what will be passed to readdir callback as the
name length.  8Kb __copy_to_user() from a buffer allocated by
__get_free_page()

Cc: stable@vger.kernel.org # 0.98pl6+ (yes, really)
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
(cherry picked from commit 99d825822eade8d827a1817357cbf3f889a552d6)
CVE-2016-4913
BugLink: https://bugs.launchpad.net/bugs/1583962
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
 fs/isofs/rock.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Tim Gardner May 25, 2016, 3:27 p.m. UTC | #1

Kamal Mostafa May 25, 2016, 6:30 p.m. UTC | #2

Chris J Arges May 26, 2016, 12:52 p.m. UTC | #3
On Wed, May 25, 2016 at 04:22:38PM +0100, Luis Henriques wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> Payloads of NM entries are not supposed to contain NUL.  When we run
> into such, only the part prior to the first NUL goes into the
> concatenation (i.e. the directory entry name being encoded by a bunch
> of NM entries).  We do stop when the amount collected so far + the
> claimed amount in the current NM entry exceed 254.  So far, so good,
> but what we return as the total length is the sum of *claimed*
> sizes, not the actual amount collected.  And that can grow pretty
> large - not unlimited, since you'd need to put CE entries in
> between to be able to get more than the maximum that could be
> contained in one isofs directory entry / continuation chunk and
> we are stop once we'd encountered 32 CEs, but you can get about 8Kb
> easily.  And that's what will be passed to readdir callback as the
> name length.  8Kb __copy_to_user() from a buffer allocated by
> __get_free_page()
> 
> Cc: stable@vger.kernel.org # 0.98pl6+ (yes, really)
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> (cherry picked from commit 99d825822eade8d827a1817357cbf3f889a552d6)
> CVE-2016-4913
> BugLink: https://bugs.launchpad.net/bugs/1583962
> Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
> ---
>  fs/isofs/rock.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/isofs/rock.c b/fs/isofs/rock.c
> index 17809499c752..e52a1ac168ef 100644
> --- a/fs/isofs/rock.c
> +++ b/fs/isofs/rock.c
> @@ -203,6 +203,8 @@ int get_rock_ridge_filename(struct iso_directory_record *de,
>  	int retnamlen = 0;
>  	int truncate = 0;
>  	int ret = 0;
> +	char *p;
> +	int len;
>  
>  	if (!ISOFS_SB(inode->i_sb)->s_rock)
>  		return 0;
> @@ -267,12 +269,17 @@ repeat:
>  					rr->u.NM.flags);
>  				break;
>  			}
> -			if ((strlen(retname) + rr->len - 5) >= 254) {
> +			len = rr->len - 5;
> +			if (retnamlen + len >= 254) {
>  				truncate = 1;
>  				break;
>  			}
> -			strncat(retname, rr->u.NM.name, rr->len - 5);
> -			retnamlen += rr->len - 5;
> +			p = memchr(rr->u.NM.name, '\0', len);
> +			if (unlikely(p))
> +				len = p - rr->u.NM.name;
> +			memcpy(retname + retnamlen, rr->u.NM.name, len);
> +			retnamlen += len;
> +			retname[retnamlen] = '\0';
>  			break;
>  		case SIG('R', 'E'):
>  			kfree(rs.buffer);
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox

Patch

diff --git a/fs/isofs/rock.c b/fs/isofs/rock.c
index 17809499c752..e52a1ac168ef 100644
--- a/fs/isofs/rock.c
+++ b/fs/isofs/rock.c
@@ -203,6 +203,8 @@  int get_rock_ridge_filename(struct iso_directory_record *de,
 	int retnamlen = 0;
 	int truncate = 0;
 	int ret = 0;
+	char *p;
+	int len;
 
 	if (!ISOFS_SB(inode->i_sb)->s_rock)
 		return 0;
@@ -267,12 +269,17 @@  repeat:
 					rr->u.NM.flags);
 				break;
 			}
-			if ((strlen(retname) + rr->len - 5) >= 254) {
+			len = rr->len - 5;
+			if (retnamlen + len >= 254) {
 				truncate = 1;
 				break;
 			}
-			strncat(retname, rr->u.NM.name, rr->len - 5);
-			retnamlen += rr->len - 5;
+			p = memchr(rr->u.NM.name, '\0', len);
+			if (unlikely(p))
+				len = p - rr->u.NM.name;
+			memcpy(retname + retnamlen, rr->u.NM.name, len);
+			retnamlen += len;
+			retname[retnamlen] = '\0';
 			break;
 		case SIG('R', 'E'):
 			kfree(rs.buffer);