[musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab

[musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab

[Matthias Goergens]

From glibc's documentation:

> Since fields in the mtab and fstab files are separated by whitespace,
> octal  escapes  are  used  to  represent  the characters space (\040),
> tab (\011), newline (\012), and backslash (\\) in those files when they
> occur  in  one  of  the  four strings  in  a  mntent  structure.  The
> routines addmntent() and getmntent() will convert from string
> representation to escaped representation and back.  When converting
> from  escaped  representation,  the sequence \134 is also converted to a
> backslash.

This fixes the issue reported in https://www.openwall.com/lists/musl/2021/12/14/1

Please pardon the previous broken patch that I tried to send directly
via gmail.

This is my first time contributing to musl.  Please point out any ways
to improve.  I probably got a few things wrong?

Thanks!

Addendum: this is a new version.  The first one did not copy the final
null-byte.
---
 src/misc/mntent.c | 75 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 68 insertions(+), 7 deletions(-)

diff --git a/src/misc/mntent.c b/src/misc/mntent.c
index d404fbe3..49f4e386 100644
--- a/src/misc/mntent.c
+++ b/src/misc/mntent.c
@@ -1,3 +1,4 @@
+#include <assert.h>
 #include <stdio.h>
 #include <string.h>
 #include <mntent.h>
@@ -20,6 +21,37 @@ int endmntent(FILE *f)
 	return 1;
 }
 
+static inline int decode1(char** in_buf, char** out_buf,  const char* from, const char to) {
+	if(strncmp(from, *in_buf, strlen(from)) == 0) {
+		*(*out_buf)++ = to;
+		*in_buf += strlen(from);
+		return 1;
+	}
+	return 0;
+}
+
+static inline char* decode(char* buf) {
+	assert(buf != NULL);
+	char* read_cursor = buf;
+	char* write_cursor = buf;
+	while(*read_cursor) {
+		// space
+		decode1(&read_cursor, &write_cursor, "\\040", '\040')
+		// tab
+		|| decode1(&read_cursor, &write_cursor, "\\011", '\011')
+		// newline
+		|| decode1(&read_cursor, &write_cursor, "\\012", '\012')
+		// backslash
+		|| decode1(&read_cursor, &write_cursor, "\\134", '\134')
+		|| decode1(&read_cursor, &write_cursor, "\\\\", '\\')
+		// default: copy char as is.
+		|| (*write_cursor++ = *read_cursor++);
+	}
+	*write_cursor = *read_cursor;
+
+	return buf;
+}
+
 struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int buflen)
 {
 	int n[8], use_internal = (linebuf == SENTINEL);
@@ -55,10 +87,10 @@ struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int bufle
 	linebuf[n[5]] = 0;
 	linebuf[n[7]] = 0;
 
-	mnt->mnt_fsname = linebuf+n[0];
-	mnt->mnt_dir = linebuf+n[2];
-	mnt->mnt_type = linebuf+n[4];
-	mnt->mnt_opts = linebuf+n[6];
+	mnt->mnt_fsname = decode(linebuf+n[0]);
+	mnt->mnt_dir = decode(linebuf+n[2]);
+	mnt->mnt_type = decode(linebuf+n[4]);
+	mnt->mnt_opts = decode(linebuf+n[6]);
 
 	return mnt;
 }
@@ -69,12 +101,41 @@ struct mntent *getmntent(FILE *f)
 	return getmntent_r(f, &mnt, SENTINEL, 0);
 }
 
+static inline int write_string(FILE *f, const char* str)
+{
+	char c;
+	int error_occured = 0;
+	while(str && !error_occured && (c = *str++) != 0) {
+		if(c == '\040') // space
+			error_occured = fprintf(f, "%s", "\\040") < 0;
+		else if (c == '\011') // tab
+			error_occured = fprintf(f, "%s", "\\011") < 0;
+		else if (c == '\012') // newline
+			error_occured = fprintf(f, "%s", "\\012") < 0;
+		else if (c == '\\')
+			error_occured = fprintf(f, "%s", "\\\\") < 0;
+		else
+			error_occured = fprintf(f, "%c", c) < 0;
+	}
+	return error_occured;
+}
+
 int addmntent(FILE *f, const struct mntent *mnt)
 {
 	if (fseek(f, 0, SEEK_END)) return 1;
-	return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
-		mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
-		mnt->mnt_freq, mnt->mnt_passno) < 0;
+	flockfile(f);
+	int result =
+		write_string(f, mnt->mnt_fsname)
+		|| (fprintf(f, "\t") < 0)
+		|| write_string(f, mnt->mnt_dir)
+		|| (fprintf(f, "\t") < 0)
+		|| write_string(f, mnt->mnt_type)
+		|| (fprintf(f, "\t") < 0)
+		|| write_string(f, mnt->mnt_opts)
+		|| (fprintf(f, "\t%d\t%d\n",
+			mnt->mnt_freq, mnt->mnt_passno) < 0);
+	funlockfile(f);
+	return result;
 }
 
 char *hasmntopt(const struct mntent *mnt, const char *opt)
-- 
2.40.0

Re: [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab

[Szabolcs Nagy]

* Matthias Goergens <matthias.goergens@gmail.com> [2023-03-29 23:17:51 +0800]:

> >From glibc's documentation:
> 
> > Since fields in the mtab and fstab files are separated by whitespace,
> > octal  escapes  are  used  to  represent  the characters space (\040),
> > tab (\011), newline (\012), and backslash (\\) in those files when they
> > occur  in  one  of  the  four strings  in  a  mntent  structure.  The
> > routines addmntent() and getmntent() will convert from string
> > representation to escaped representation and back.  When converting
> > from  escaped  representation,  the sequence \134 is also converted to a
> > backslash.
> 
> This fixes the issue reported in https://www.openwall.com/lists/musl/2021/12/14/1
> 
> Please pardon the previous broken patch that I tried to send directly
> via gmail.
> 
> This is my first time contributing to musl.  Please point out any ways
> to improve.  I probably got a few things wrong?
> 
> Thanks!
> 
> Addendum: this is a new version.  The first one did not copy the final
> null-byte.
> ---
>  src/misc/mntent.c | 75 ++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 68 insertions(+), 7 deletions(-)
> 
> diff --git a/src/misc/mntent.c b/src/misc/mntent.c
> index d404fbe3..49f4e386 100644
> --- a/src/misc/mntent.c
> +++ b/src/misc/mntent.c
> @@ -1,3 +1,4 @@
> +#include <assert.h>
>  #include <stdio.h>
>  #include <string.h>
>  #include <mntent.h>
> @@ -20,6 +21,37 @@ int endmntent(FILE *f)
>  	return 1;
>  }
>  
> +static inline int decode1(char** in_buf, char** out_buf,  const char* from, const char to) {
> +	if(strncmp(from, *in_buf, strlen(from)) == 0) {
> +		*(*out_buf)++ = to;
> +		*in_buf += strlen(from);
> +		return 1;
> +	}
> +	return 0;
> +}
> +
> +static inline char* decode(char* buf) {
> +	assert(buf != NULL);
> +	char* read_cursor = buf;
> +	char* write_cursor = buf;
> +	while(*read_cursor) {
> +		// space
> +		decode1(&read_cursor, &write_cursor, "\\040", '\040')
> +		// tab
> +		|| decode1(&read_cursor, &write_cursor, "\\011", '\011')
> +		// newline
> +		|| decode1(&read_cursor, &write_cursor, "\\012", '\012')
> +		// backslash
> +		|| decode1(&read_cursor, &write_cursor, "\\134", '\134')
> +		|| decode1(&read_cursor, &write_cursor, "\\\\", '\\')
> +		// default: copy char as is.
> +		|| (*write_cursor++ = *read_cursor++);
> +	}

this will try matching at every position, but
we know that each escape starts with \ so this
can be optimized.

i expect inlining decode1 will increase code size
unnecessarily. you can avoid that e.g. like

for (;;) {
	...
	const char *replace =
		"\040" "040" "\0"
		"\011" "011" "\0"
		...
		"\\" "\\" "\0"
		"\\" "";
	for (;;) {
		char c = *replace++;
		size_t n = strlen(replace);
		if (strncmp(pr, replace, n) == 0) {
			*pw++ = c;
			pr += n;
			break;
		}
		replace += n+1;
	}
	...
	char *next = __strchrnul(pr, '\\');
	memmove(pw, pr, next-pr);
	...
}

> +	*write_cursor = *read_cursor;
> +
> +	return buf;
> +}
> +
>  struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int buflen)
>  {
>  	int n[8], use_internal = (linebuf == SENTINEL);
> @@ -55,10 +87,10 @@ struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int bufle
>  	linebuf[n[5]] = 0;
>  	linebuf[n[7]] = 0;
>  
> -	mnt->mnt_fsname = linebuf+n[0];
> -	mnt->mnt_dir = linebuf+n[2];
> -	mnt->mnt_type = linebuf+n[4];
> -	mnt->mnt_opts = linebuf+n[6];
> +	mnt->mnt_fsname = decode(linebuf+n[0]);
> +	mnt->mnt_dir = decode(linebuf+n[2]);
> +	mnt->mnt_type = decode(linebuf+n[4]);
> +	mnt->mnt_opts = decode(linebuf+n[6]);
>  
>  	return mnt;
>  }
> @@ -69,12 +101,41 @@ struct mntent *getmntent(FILE *f)
>  	return getmntent_r(f, &mnt, SENTINEL, 0);
>  }
>  
> +static inline int write_string(FILE *f, const char* str)
> +{
> +	char c;
> +	int error_occured = 0;
> +	while(str && !error_occured && (c = *str++) != 0) {
> +		if(c == '\040') // space
> +			error_occured = fprintf(f, "%s", "\\040") < 0;
> +		else if (c == '\011') // tab
> +			error_occured = fprintf(f, "%s", "\\011") < 0;
> +		else if (c == '\012') // newline
> +			error_occured = fprintf(f, "%s", "\\012") < 0;
> +		else if (c == '\\')
> +			error_occured = fprintf(f, "%s", "\\\\") < 0;
> +		else
> +			error_occured = fprintf(f, "%c", c) < 0;
> +	}
> +	return error_occured;
> +}
> +
>  int addmntent(FILE *f, const struct mntent *mnt)
>  {
>  	if (fseek(f, 0, SEEK_END)) return 1;
> -	return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
> -		mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
> -		mnt->mnt_freq, mnt->mnt_passno) < 0;

the original code has a bug here: fprintf returns number of
chars printed but addmntent returns 0 to indicate success.

i think this should be fixed independently of the new feature.
(separate patch).

> +	flockfile(f);
> +	int result =
> +		write_string(f, mnt->mnt_fsname)
> +		|| (fprintf(f, "\t") < 0)
> +		|| write_string(f, mnt->mnt_dir)
> +		|| (fprintf(f, "\t") < 0)
> +		|| write_string(f, mnt->mnt_type)
> +		|| (fprintf(f, "\t") < 0)
> +		|| write_string(f, mnt->mnt_opts)
> +		|| (fprintf(f, "\t%d\t%d\n",
> +			mnt->mnt_freq, mnt->mnt_passno) < 0);
> +	funlockfile(f);

this looks a bit ugly (recursive locks and char-by-char
printf) but i don't see anything wrong (again i would not
use inline).

> +	return result;
>  }
>  
>  char *hasmntopt(const struct mntent *mnt, const char *opt)
> -- 
> 2.40.0

Re: [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2023-03-29 19:04:18 +0200]:
> > -	return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
> > -		mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
> > -		mnt->mnt_freq, mnt->mnt_passno) < 0;
> 
> the original code has a bug here: fprintf returns number of
> chars printed but addmntent returns 0 to indicate success.

sorry missed the < 0.

Re: [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab

[Matthias Görgens]

On Thu, 30 Mar 2023, 01:16 Szabolcs Nagy, <nsz@port70.net> wrote:
>
> * Szabolcs Nagy <nsz@port70.net> [2023-03-29 19:04:18 +0200]:
> > > -   return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
> > > -           mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
> > > -           mnt->mnt_freq, mnt->mnt_passno) < 0;
> >
> > the original code has a bug here: fprintf returns number of
> > chars printed but addmntent returns 0 to indicate success.
>
> sorry missed the < 0.

I made that exact same error when first reading the code.  I wonder whether we
should put the zero in front, to make it clearer to readers? Like `0 >
fprintf(...`

Re: [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab

[Pascal Cuoq]

>> sorry missed the < 0.
> 
> I made that exact same error when first reading the code.  I wonder whether we
> should put the zero in front, to make it clearer to readers? Like `0 >
> fprintf(...`

As someone who reviews tons of C code from different authors and application domains, I expect the result of fprintf to be assigned to a variable and that variable to be used in the return statement. Anything else will give me pause or be confused for something that works when it doesn't.

Pascal

Re: [musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab

[Rich Felker]

On Thu, Mar 30, 2023 at 09:29:12AM +0000, Pascal Cuoq wrote:
> 
> 
> >> sorry missed the < 0.
> > 
> > I made that exact same error when first reading the code.  I wonder whether we
> > should put the zero in front, to make it clearer to readers? Like `0 >
> > fprintf(...`
> 
> As someone who reviews tons of C code from different authors and
> application domains, I expect the result of fprintf to be assigned
> to a variable and that variable to be used in the return statement.
> Anything else will give me pause or be confused for something that
> works when it doesn't.

OK, but that's not generally a style we use in musl (introducing vars
to save a value that will only be used once immediately after it's
stored).

[musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab

[Matthias Goergens]

From glibc's documentation:

> Since fields in the mtab and fstab files are separated by whitespace,
> octal  escapes  are  used  to  represent  the characters space (\040),
> tab (\011), newline (\012), and backslash (\\) in those files when they
> occur  in  one  of  the  four strings  in  a  mntent  structure.  The
> routines addmntent() and getmntent() will convert from string
> representation to escaped representation and back.  When converting
> from  escaped  representation,  the sequence \134 is also converted to a
> backslash.

This fixes the issue reported in https://www.openwall.com/lists/musl/2021/12/14/1

--

This is a new version that incorporates suggestions by Szabolcs Nagy and rofl0r

The previous version had a helper function named `decode1` that decoded
a single escape.   I implemented Szabolcs Nagy's extremely clever manual
inlining.  However, I'm not completely sure that it's not too clever for
me.  (It took me a while to understand why it works.)
---
 src/misc/mntent.c | 85 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 78 insertions(+), 7 deletions(-)

diff --git a/src/misc/mntent.c b/src/misc/mntent.c
index d404fbe3..1c129579 100644
--- a/src/misc/mntent.c
+++ b/src/misc/mntent.c
@@ -1,8 +1,10 @@
+#include <assert.h>
 #include <stdio.h>
 #include <string.h>
 #include <mntent.h>
 #include <errno.h>
 #include <limits.h>
+#include "stdio_impl.h"
 
 static char *internal_buf;
 static size_t internal_bufsize;
@@ -20,6 +22,46 @@ int endmntent(FILE *f)
 	return 1;
 }
 
+static char* decode(char* buf) {
+	assert(buf != NULL);
+	char* src = buf;
+	char* dest = buf;
+	while (1) {
+		char* next_src = __strchrnul(src, '\\');
+		int offset = next_src - src;
+		memmove(dest, src, offset);
+		src = next_src;
+		dest += offset;
+
+		if(*src == '\0') {
+			*dest = *src;
+			return buf;
+		}
+		assert (*src == '\\');
+		src++;
+
+		const char *replacements =
+			"\040"	"040"	"\0"  // space
+			"\011"	"011"	"\0"  // tab
+			"\012"	"012"	"\0"  // newline
+			"\134"	"134"	"\0"  // backslash
+			"\\"	"\\"	"\0"
+			// Fallback for unrecognized escape sequence,
+			// copy literally:
+			"\\"	"";
+		while(1) {
+			char c = *replacements++;
+			size_t n = strlen(replacements);
+			if (strncmp(src, replacements, n) == 0) {
+				*dest++ = c;
+				src += n;
+				break;
+			}
+			replacements += n+1;
+        }
+	}
+}
+
 struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int buflen)
 {
 	int n[8], use_internal = (linebuf == SENTINEL);
@@ -55,10 +97,10 @@ struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int bufle
 	linebuf[n[5]] = 0;
 	linebuf[n[7]] = 0;
 
-	mnt->mnt_fsname = linebuf+n[0];
-	mnt->mnt_dir = linebuf+n[2];
-	mnt->mnt_type = linebuf+n[4];
-	mnt->mnt_opts = linebuf+n[6];
+	mnt->mnt_fsname = decode(linebuf+n[0]);
+	mnt->mnt_dir = decode(linebuf+n[2]);
+	mnt->mnt_type = decode(linebuf+n[4]);
+	mnt->mnt_opts = decode(linebuf+n[6]);
 
 	return mnt;
 }
@@ -69,12 +111,41 @@ struct mntent *getmntent(FILE *f)
 	return getmntent_r(f, &mnt, SENTINEL, 0);
 }
 
+static int escape_and_write_string(FILE *f, const char* str)
+{
+	char c;
+	int error_occured = 0;
+	while(str && !error_occured && (c = *str++) != 0) {
+		if(c == '\040') // space
+			error_occured = fputs("\\040", f) < 0;
+		else if (c == '\011') // tab
+			error_occured = fputs("\\011", f) < 0;
+		else if (c == '\012') // newline
+			error_occured = fputs("\\012", f) < 0;
+		else if (c == '\\')
+			error_occured = fputs("\\\\", f) < 0;
+		else
+			error_occured = putc_unlocked(c, f) < 0;
+	}
+	return error_occured;
+}
+
 int addmntent(FILE *f, const struct mntent *mnt)
 {
 	if (fseek(f, 0, SEEK_END)) return 1;
-	return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
-		mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
-		mnt->mnt_freq, mnt->mnt_passno) < 0;
+	FLOCK(f);
+	int error_occured =
+		escape_and_write_string(f, mnt->mnt_fsname)
+		|| (0 > putc_unlocked('\t', f))
+		|| escape_and_write_string(f, mnt->mnt_dir)
+		|| (0 > putc_unlocked('\t', f))
+		|| escape_and_write_string(f, mnt->mnt_type)
+		|| (0 > putc_unlocked('\t', f))
+		|| escape_and_write_string(f, mnt->mnt_opts)
+		|| (0 > fprintf(f, "\t%d\t%d\n",
+			mnt->mnt_freq, mnt->mnt_passno));
+	FUNLOCK(f);
+	return error_occured;
 }
 
 char *hasmntopt(const struct mntent *mnt, const char *opt)
-- 
2.40.0

[musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab

[Matthias Goergens]

From glibc's documentation:

> Since fields in the mtab and fstab files are separated by whitespace,
> octal  escapes  are  used  to  represent  the characters space (\040),
> tab (\011), newline (\012), and backslash (\\) in those files when they
> occur  in  one  of  the  four strings  in  a  mntent  structure.  The
> routines addmntent() and getmntent() will convert from string
> representation to escaped representation and back.  When converting
> from  escaped  representation,  the sequence \134 is also converted to a
> backslash.

This fixes the issue reported in https://www.openwall.com/lists/musl/2021/12/14/1

Please pardon the previous broken patch that I tried to send directly
via gmail.

This is my first time contributing to musl.  Please point out any ways
to improve.  I probably got a few things wrong?

Thanks!
---
 src/misc/mntent.c | 71 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 64 insertions(+), 7 deletions(-)

diff --git a/src/misc/mntent.c b/src/misc/mntent.c
index d404fbe3..527acc01 100644
--- a/src/misc/mntent.c
+++ b/src/misc/mntent.c
@@ -20,6 +20,34 @@ int endmntent(FILE *f)
 	return 1;
 }
 
+static inline int decode1(char** in_buf, char** out_buf,  const char* from, const char to) {
+	if(strncmp(from, *in_buf, strlen(from)) == 0) {
+		*(*out_buf)++ = to;
+		*in_buf += strlen(from);
+		return 1;
+	}
+	return 0;
+}
+
+static inline char* decode(char* buf) {
+	char* read_cursor = buf;
+	char* write_cursor = buf;
+	while(read_cursor && *read_cursor) {
+			// space
+			decode1(&read_cursor, &write_cursor, "\\040", '\040')
+			// tab
+			|| decode1(&read_cursor, &write_cursor, "\\011", '\011')
+			// newline
+			|| decode1(&read_cursor, &write_cursor, "\\012", '\012')
+			// backslash
+			|| decode1(&read_cursor, &write_cursor, "\\134", '\134')
+			|| decode1(&read_cursor, &write_cursor, "\\\\", '\\')
+			// default: copy char as is.
+			|| (*write_cursor++ = *read_cursor++);
+	}
+	return buf;
+}
+
 struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int buflen)
 {
 	int n[8], use_internal = (linebuf == SENTINEL);
@@ -55,10 +83,10 @@ struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf, int bufle
 	linebuf[n[5]] = 0;
 	linebuf[n[7]] = 0;
 
-	mnt->mnt_fsname = linebuf+n[0];
-	mnt->mnt_dir = linebuf+n[2];
-	mnt->mnt_type = linebuf+n[4];
-	mnt->mnt_opts = linebuf+n[6];
+	mnt->mnt_fsname = decode(linebuf+n[0]);
+	mnt->mnt_dir = decode(linebuf+n[2]);
+	mnt->mnt_type = decode(linebuf+n[4]);
+	mnt->mnt_opts = decode(linebuf+n[6]);
 
 	return mnt;
 }
@@ -69,12 +97,41 @@ struct mntent *getmntent(FILE *f)
 	return getmntent_r(f, &mnt, SENTINEL, 0);
 }
 
+static inline int write_string(FILE *f, const char* str)
+{
+	char c;
+	int error_occured = 0;
+	while(str && !error_occured && (c = *str++) != 0) {
+		if(c == '\040') // space
+			error_occured = fprintf(f, "%s", "\\040") < 0;
+		else if (c == '\011') // tab
+			error_occured = fprintf(f, "%s", "\\011") < 0;
+		else if (c == '\012') // newline
+			error_occured = fprintf(f, "%s", "\\012") < 0;
+		else if (c == '\\')
+			error_occured = fprintf(f, "%s", "\\\\") < 0;
+		else
+			error_occured = fprintf(f, "%c", c) < 0;
+	}
+	return error_occured;
+}
+
 int addmntent(FILE *f, const struct mntent *mnt)
 {
 	if (fseek(f, 0, SEEK_END)) return 1;
-	return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
-		mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
-		mnt->mnt_freq, mnt->mnt_passno) < 0;
+	flockfile(f);
+	int result =
+		write_string(f, mnt->mnt_fsname)
+		|| (fprintf(f, "\t") < 0)
+		|| write_string(f, mnt->mnt_dir)
+		|| (fprintf(f, "\t") < 0)
+		|| write_string(f, mnt->mnt_type)
+		|| (fprintf(f, "\t") < 0)
+		|| write_string(f, mnt->mnt_opts)
+		|| (fprintf(f, "\t%d\t%d\n",
+			mnt->mnt_freq, mnt->mnt_passno) < 0);
+	funlockfile(f);
+	return result;
 }
 
 char *hasmntopt(const struct mntent *mnt, const char *opt)
-- 
2.40.0

[musl] [PATCH] mntent: deal with escaped whitespace in mtab and fstab

[Matthias Görgens]

From ca1001f856e314b0ea9d0f3a5d7c88106f151c2e Mon Sep 17 00:00:00 2001
From: Matthias Goergens <matthias.goergens@gmail.com>
Date: Wed, 29 Mar 2023 16:17:49 +0800
Subject: [PATCH] mntent: deal with escaped whitespace in mtab and fstab

From glibc's documentation:

> Since fields in the mtab and fstab files are separated by whitespace,
> octal escapes are used to represent the characters space (\040),
> tab (\011), newline (\012), and backslash (\\) in those files when they
> occur in one of the four strings in a mntent structure. The
> routines addmntent() and getmntent() will convert from string
> representation to escaped representation and back. When converting
> from escaped representation, the sequence \134 is also converted to a
> backslash.

This fixes the issue reported in
https://www.openwall.com/lists/musl/2021/12/14/1

This is my first time contributing to musl.  Please point out any
mistakes you can find.

I am not quite sure I understand everything, and have also probably
gotten the style wrong?

Thanks!
---
src/misc/mntent.c | 71 ++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 64 insertions(+), 7 deletions(-)

diff --git a/src/misc/mntent.c b/src/misc/mntent.c
index d404fbe3..527acc01 100644
--- a/src/misc/mntent.c
+++ b/src/misc/mntent.c
@@ -20,6 +20,34 @@ int endmntent(FILE *f)
return 1;
}
+static inline int decode1(char** in_buf, char** out_buf, const char*
from, const char to) {
+ if(strncmp(from, *in_buf, strlen(from)) == 0) {
+ *(*out_buf)++ = to;
+ *in_buf += strlen(from);
+ return 1;
+ }
+ return 0;
+}
+
+static inline char* decode(char* buf) {
+ char* read_cursor = buf;
+ char* write_cursor = buf;
+ while(read_cursor && *read_cursor) {
+ // space
+ decode1(&read_cursor, &write_cursor, "\\040", '\040')
+ // tab
+ || decode1(&read_cursor, &write_cursor, "\\011", '\011')
+ // newline
+ || decode1(&read_cursor, &write_cursor, "\\012", '\012')
+ // backslash
+ || decode1(&read_cursor, &write_cursor, "\\134", '\134')
+ || decode1(&read_cursor, &write_cursor, "\\\\", '\\')
+ // default: copy char as is.
+ || (*write_cursor++ = *read_cursor++);
+ }
+ return buf;
+}
+
struct mntent *getmntent_r(FILE *f, struct mntent *mnt, char *linebuf,
int buflen)
{
int n[8], use_internal = (linebuf == SENTINEL);
@@ -55,10 +83,10 @@ struct mntent *getmntent_r(FILE *f, struct mntent
*mnt, char *linebuf, int bufle
linebuf[n[5]] = 0;
linebuf[n[7]] = 0;
- mnt->mnt_fsname = linebuf+n[0];
- mnt->mnt_dir = linebuf+n[2];
- mnt->mnt_type = linebuf+n[4];
- mnt->mnt_opts = linebuf+n[6];
+ mnt->mnt_fsname = decode(linebuf+n[0]);
+ mnt->mnt_dir = decode(linebuf+n[2]);
+ mnt->mnt_type = decode(linebuf+n[4]);
+ mnt->mnt_opts = decode(linebuf+n[6]);
return mnt;
}
@@ -69,12 +97,41 @@ struct mntent *getmntent(FILE *f)
return getmntent_r(f, &mnt, SENTINEL, 0);
}
+static inline int write_string(FILE *f, const char* str)
+{
+ char c;
+ int error_occured = 0;
+ while(str && !error_occured && (c = *str++) != 0) {
+ if(c == '\040') // space
+ error_occured = fprintf(f, "%s", "\\040") < 0;
+ else if (c == '\011') // tab
+ error_occured = fprintf(f, "%s", "\\011") < 0;
+ else if (c == '\012') // newline
+ error_occured = fprintf(f, "%s", "\\012") < 0;
+ else if (c == '\\')
+ error_occured = fprintf(f, "%s", "\\\\") < 0;
+ else
+ error_occured = fprintf(f, "%c", c) < 0;
+ }
+ return error_occured;
+}
+
int addmntent(FILE *f, const struct mntent *mnt)
{
if (fseek(f, 0, SEEK_END)) return 1;
- return fprintf(f, "%s\t%s\t%s\t%s\t%d\t%d\n",
- mnt->mnt_fsname, mnt->mnt_dir, mnt->mnt_type, mnt->mnt_opts,
- mnt->mnt_freq, mnt->mnt_passno) < 0;
+ flockfile(f);
+ int result =
+ write_string(f, mnt->mnt_fsname)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_dir)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_type)
+ || (fprintf(f, "\t") < 0)
+ || write_string(f, mnt->mnt_opts)
+ || (fprintf(f, "\t%d\t%d\n",
+ mnt->mnt_freq, mnt->mnt_passno) < 0);
+ funlockfile(f);
+ return result;
}
char *hasmntopt(const struct mntent *mnt, const char *opt)
-- 
2.40.0