* [OE-core][hardknott][PATCH] cpio: fix CVE-2021-38185
@ 2021-09-13 4:49 Chen Qi
2021-09-13 7:11 ` Anuj Mittal
0 siblings, 1 reply; 5+ messages in thread
From: Chen Qi @ 2021-09-13 4:49 UTC (permalink / raw)
To: openembedded-core
Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
.../0001-Rewrite-dynamic-string-support.patch | 458 ++++++++++++++++++
meta/recipes-extended/cpio/cpio_2.13.bb | 1 +
2 files changed, 459 insertions(+)
create mode 100644 meta/recipes-extended/cpio/cpio-2.13/0001-Rewrite-dynamic-string-support.patch
diff --git a/meta/recipes-extended/cpio/cpio-2.13/0001-Rewrite-dynamic-string-support.patch b/meta/recipes-extended/cpio/cpio-2.13/0001-Rewrite-dynamic-string-support.patch
new file mode 100644
index 0000000000..93ed05184f
--- /dev/null
+++ b/meta/recipes-extended/cpio/cpio-2.13/0001-Rewrite-dynamic-string-support.patch
@@ -0,0 +1,458 @@
+From 0cdda8baddbbc50a4212e36a470053ac624075bb Mon Sep 17 00:00:00 2001
+From: Sergey Poznyakoff <gray@gnu.org>
+Date: Sat, 7 Aug 2021 12:52:21 +0300
+Subject: [PATCH] Rewrite dynamic string support.
+
+* src/dstring.c (ds_init): Take a single argument.
+(ds_free): New function.
+(ds_resize): Take a single argument. Use x2nrealloc to expand
+the storage.
+(ds_reset,ds_append,ds_concat,ds_endswith): New function.
+(ds_fgetstr): Rewrite. In particular, this fixes integer overflow.
+* src/dstring.h (dynamic_string): Keep both the allocated length
+(ds_size) and index of the next free byte in the string (ds_idx).
+(ds_init,ds_resize): Change signature.
+(ds_len): New macro.
+(ds_free,ds_reset,ds_append,ds_concat,ds_endswith): New protos.
+* src/copyin.c: Use new ds_ functions.
+* src/copyout.c: Likewise.
+* src/copypass.c: Likewise.
+* src/util.c: Likewise.
+
+Upstream-Status: Backport
+CVE: CVE-2021-38185
+Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
+---
+ src/copyin.c | 40 +++++++++++------------
+ src/copyout.c | 16 ++++-----
+ src/copypass.c | 34 +++++++++----------
+ src/dstring.c | 88 ++++++++++++++++++++++++++++++++++++--------------
+ src/dstring.h | 31 +++++++++---------
+ src/util.c | 6 ++--
+ 6 files changed, 123 insertions(+), 92 deletions(-)
+
+diff --git a/src/copyin.c b/src/copyin.c
+index b29f348..37e503a 100644
+--- a/src/copyin.c
++++ b/src/copyin.c
+@@ -55,11 +55,12 @@ query_rename(struct cpio_file_stat* file_hdr, FILE *tty_in, FILE *tty_out,
+ char *str_res; /* Result for string function. */
+ static dynamic_string new_name; /* New file name for rename option. */
+ static int initialized_new_name = false;
++
+ if (!initialized_new_name)
+- {
+- ds_init (&new_name, 128);
+- initialized_new_name = true;
+- }
++ {
++ ds_init (&new_name);
++ initialized_new_name = true;
++ }
+
+ if (rename_flag)
+ {
+@@ -779,37 +780,36 @@ long_format (struct cpio_file_stat *file_hdr, char const *link_name)
+ already in `save_patterns' (from the command line) are preserved. */
+
+ static void
+-read_pattern_file ()
++read_pattern_file (void)
+ {
+- int max_new_patterns;
+- char **new_save_patterns;
+- int new_num_patterns;
++ char **new_save_patterns = NULL;
++ size_t max_new_patterns;
++ size_t new_num_patterns;
+ int i;
+- dynamic_string pattern_name;
++ dynamic_string pattern_name = DYNAMIC_STRING_INITIALIZER;
+ FILE *pattern_fp;
+
+ if (num_patterns < 0)
+ num_patterns = 0;
+- max_new_patterns = 1 + num_patterns;
+- new_save_patterns = (char **) xmalloc (max_new_patterns * sizeof (char *));
+ new_num_patterns = num_patterns;
+- ds_init (&pattern_name, 128);
++ max_new_patterns = num_patterns;
++ new_save_patterns = xcalloc (max_new_patterns, sizeof (new_save_patterns[0]));
+
+ pattern_fp = fopen (pattern_file_name, "r");
+ if (pattern_fp == NULL)
+ open_fatal (pattern_file_name);
+ while (ds_fgetstr (pattern_fp, &pattern_name, '\n') != NULL)
+ {
+- if (new_num_patterns >= max_new_patterns)
+- {
+- max_new_patterns += 1;
+- new_save_patterns = (char **)
+- xrealloc ((char *) new_save_patterns,
+- max_new_patterns * sizeof (char *));
+- }
++ if (new_num_patterns == max_new_patterns)
++ new_save_patterns = x2nrealloc (new_save_patterns,
++ &max_new_patterns,
++ sizeof (new_save_patterns[0]));
+ new_save_patterns[new_num_patterns] = xstrdup (pattern_name.ds_string);
+ ++new_num_patterns;
+ }
++
++ ds_free (&pattern_name);
++
+ if (ferror (pattern_fp) || fclose (pattern_fp) == EOF)
+ close_error (pattern_file_name);
+
+@@ -1196,7 +1196,7 @@ swab_array (char *ptr, int count)
+ in the file system. */
+
+ void
+-process_copy_in ()
++process_copy_in (void)
+ {
+ char done = false; /* True if trailer reached. */
+ FILE *tty_in = NULL; /* Interactive file for rename option. */
+diff --git a/src/copyout.c b/src/copyout.c
+index 8b0beb6..26e3dda 100644
+--- a/src/copyout.c
++++ b/src/copyout.c
+@@ -594,9 +594,10 @@ assign_string (char **pvar, char *value)
+ The format of the header depends on the compatibility (-c) flag. */
+
+ void
+-process_copy_out ()
++process_copy_out (void)
+ {
+- dynamic_string input_name; /* Name of file read from stdin. */
++ dynamic_string input_name = DYNAMIC_STRING_INITIALIZER;
++ /* Name of file read from stdin. */
+ struct stat file_stat; /* Stat record for file. */
+ struct cpio_file_stat file_hdr = CPIO_FILE_STAT_INITIALIZER;
+ /* Output header information. */
+@@ -605,7 +606,6 @@ process_copy_out ()
+ char *orig_file_name = NULL;
+
+ /* Initialize the copy out. */
+- ds_init (&input_name, 128);
+ file_hdr.c_magic = 070707;
+
+ /* Check whether the output file might be a tape. */
+@@ -657,14 +657,9 @@ process_copy_out ()
+ {
+ if (file_hdr.c_mode & CP_IFDIR)
+ {
+- int len = strlen (input_name.ds_string);
+ /* Make sure the name ends with a slash */
+- if (input_name.ds_string[len-1] != '/')
+- {
+- ds_resize (&input_name, len + 2);
+- input_name.ds_string[len] = '/';
+- input_name.ds_string[len+1] = 0;
+- }
++ if (!ds_endswith (&input_name, '/'))
++ ds_append (&input_name, '/');
+ }
+ }
+
+@@ -875,6 +870,7 @@ process_copy_out ()
+ (unsigned long) blocks), (unsigned long) blocks);
+ }
+ cpio_file_stat_free (&file_hdr);
++ ds_free (&input_name);
+ }
+
+
+diff --git a/src/copypass.c b/src/copypass.c
+index dc13b5b..62f31c6 100644
+--- a/src/copypass.c
++++ b/src/copypass.c
+@@ -48,10 +48,12 @@ set_copypass_perms (int fd, const char *name, struct stat *st)
+ If `link_flag', link instead of copying. */
+
+ void
+-process_copy_pass ()
++process_copy_pass (void)
+ {
+- dynamic_string input_name; /* Name of file from stdin. */
+- dynamic_string output_name; /* Name of new file. */
++ dynamic_string input_name = DYNAMIC_STRING_INITIALIZER;
++ /* Name of file from stdin. */
++ dynamic_string output_name = DYNAMIC_STRING_INITIALIZER;
++ /* Name of new file. */
+ size_t dirname_len; /* Length of `directory_name'. */
+ int res; /* Result of functions. */
+ char *slash; /* For moving past slashes in input name. */
+@@ -65,25 +67,18 @@ process_copy_pass ()
+ created files */
+
+ /* Initialize the copy pass. */
+- ds_init (&input_name, 128);
+
+ dirname_len = strlen (directory_name);
+ if (change_directory_option && !ISSLASH (directory_name[0]))
+ {
+ char *pwd = xgetcwd ();
+-
+- dirname_len += strlen (pwd) + 1;
+- ds_init (&output_name, dirname_len + 2);
+- strcpy (output_name.ds_string, pwd);
+- strcat (output_name.ds_string, "/");
+- strcat (output_name.ds_string, directory_name);
++
++ ds_concat (&output_name, pwd);
++ ds_append (&output_name, '/');
+ }
+- else
+- {
+- ds_init (&output_name, dirname_len + 2);
+- strcpy (output_name.ds_string, directory_name);
+- }
+- output_name.ds_string[dirname_len] = '/';
++ ds_concat (&output_name, directory_name);
++ ds_append (&output_name, '/');
++ dirname_len = ds_len (&output_name);
+ output_is_seekable = true;
+
+ change_dir ();
+@@ -116,8 +111,8 @@ process_copy_pass ()
+ /* Make the name of the new file. */
+ for (slash = input_name.ds_string; *slash == '/'; ++slash)
+ ;
+- ds_resize (&output_name, dirname_len + strlen (slash) + 2);
+- strcpy (output_name.ds_string + dirname_len + 1, slash);
++ ds_reset (&output_name, dirname_len);
++ ds_concat (&output_name, slash);
+
+ existing_dir = false;
+ if (lstat (output_name.ds_string, &out_file_stat) == 0)
+@@ -333,6 +328,9 @@ process_copy_pass ()
+ (unsigned long) blocks),
+ (unsigned long) blocks);
+ }
++
++ ds_free (&input_name);
++ ds_free (&output_name);
+ }
+ \f
+ /* Try and create a hard link from FILE_NAME to another file
+diff --git a/src/dstring.c b/src/dstring.c
+index e9c063f..358f356 100644
+--- a/src/dstring.c
++++ b/src/dstring.c
+@@ -20,8 +20,8 @@
+ #if defined(HAVE_CONFIG_H)
+ # include <config.h>
+ #endif
+-
+ #include <stdio.h>
++#include <stdlib.h>
+ #if defined(HAVE_STRING_H) || defined(STDC_HEADERS)
+ #include <string.h>
+ #else
+@@ -33,24 +33,41 @@
+ /* Initialiaze dynamic string STRING with space for SIZE characters. */
+
+ void
+-ds_init (dynamic_string *string, int size)
++ds_init (dynamic_string *string)
++{
++ memset (string, 0, sizeof *string);
++}
++
++/* Free the dynamic string storage. */
++
++void
++ds_free (dynamic_string *string)
+ {
+- string->ds_length = size;
+- string->ds_string = (char *) xmalloc (size);
++ free (string->ds_string);
+ }
+
+-/* Expand dynamic string STRING, if necessary, to hold SIZE characters. */
++/* Expand dynamic string STRING, if necessary. */
+
+ void
+-ds_resize (dynamic_string *string, int size)
++ds_resize (dynamic_string *string)
+ {
+- if (size > string->ds_length)
++ if (string->ds_idx == string->ds_size)
+ {
+- string->ds_length = size;
+- string->ds_string = (char *) xrealloc ((char *) string->ds_string, size);
++ string->ds_string = x2nrealloc (string->ds_string, &string->ds_size,
++ 1);
+ }
+ }
+
++/* Reset the index of the dynamic string S to LEN. */
++
++void
++ds_reset (dynamic_string *s, size_t len)
++{
++ while (len > s->ds_size)
++ ds_resize (s);
++ s->ds_idx = len;
++}
++
+ /* Dynamic string S gets a string terminated by the EOS character
+ (which is removed) from file F. S will increase
+ in size during the function if the string from F is longer than
+@@ -61,34 +78,50 @@ ds_resize (dynamic_string *string, int size)
+ char *
+ ds_fgetstr (FILE *f, dynamic_string *s, char eos)
+ {
+- int insize; /* Amount needed for line. */
+- int strsize; /* Amount allocated for S. */
+ int next_ch;
+
+ /* Initialize. */
+- insize = 0;
+- strsize = s->ds_length;
++ s->ds_idx = 0;
+
+ /* Read the input string. */
+- next_ch = getc (f);
+- while (next_ch != eos && next_ch != EOF)
++ while ((next_ch = getc (f)) != eos && next_ch != EOF)
+ {
+- if (insize >= strsize - 1)
+- {
+- ds_resize (s, strsize * 2 + 2);
+- strsize = s->ds_length;
+- }
+- s->ds_string[insize++] = next_ch;
+- next_ch = getc (f);
++ ds_resize (s);
++ s->ds_string[s->ds_idx++] = next_ch;
+ }
+- s->ds_string[insize++] = '\0';
++ ds_resize (s);
++ s->ds_string[s->ds_idx] = '\0';
+
+- if (insize == 1 && next_ch == EOF)
++ if (s->ds_idx == 0 && next_ch == EOF)
+ return NULL;
+ else
+ return s->ds_string;
+ }
+
++void
++ds_append (dynamic_string *s, int c)
++{
++ ds_resize (s);
++ s->ds_string[s->ds_idx] = c;
++ if (c)
++ {
++ s->ds_idx++;
++ ds_resize (s);
++ s->ds_string[s->ds_idx] = 0;
++ }
++}
++
++void
++ds_concat (dynamic_string *s, char const *str)
++{
++ size_t len = strlen (str);
++ while (len + 1 > s->ds_size)
++ ds_resize (s);
++ memcpy (s->ds_string + s->ds_idx, str, len);
++ s->ds_idx += len;
++ s->ds_string[s->ds_idx] = 0;
++}
++
+ char *
+ ds_fgets (FILE *f, dynamic_string *s)
+ {
+@@ -100,3 +133,10 @@ ds_fgetname (FILE *f, dynamic_string *s)
+ {
+ return ds_fgetstr (f, s, '\0');
+ }
++
++/* Return true if the dynamic string S ends with character C. */
++int
++ds_endswith (dynamic_string *s, int c)
++{
++ return (s->ds_idx > 0 && s->ds_string[s->ds_idx - 1] == c);
++}
+diff --git a/src/dstring.h b/src/dstring.h
+index b5135fe..f5b04ef 100644
+--- a/src/dstring.h
++++ b/src/dstring.h
+@@ -17,10 +17,6 @@
+ Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ Boston, MA 02110-1301 USA. */
+
+-#ifndef NULL
+-#define NULL 0
+-#endif
+-
+ /* A dynamic string consists of record that records the size of an
+ allocated string and the pointer to that string. The actual string
+ is a normal zero byte terminated string that can be used with the
+@@ -30,22 +26,25 @@
+
+ typedef struct
+ {
+- int ds_length; /* Actual amount of storage allocated. */
+- char *ds_string; /* String. */
++ size_t ds_size; /* Actual amount of storage allocated. */
++ size_t ds_idx; /* Index of the next free byte in the string. */
++ char *ds_string; /* String storage. */
+ } dynamic_string;
+
++#define DYNAMIC_STRING_INITIALIZER { 0, 0, NULL }
+
+-/* Macros that look similar to the original string functions.
+- WARNING: These macros work only on pointers to dynamic string records.
+- If used with a real record, an "&" must be used to get the pointer. */
+-#define ds_strlen(s) strlen ((s)->ds_string)
+-#define ds_strcmp(s1, s2) strcmp ((s1)->ds_string, (s2)->ds_string)
+-#define ds_strncmp(s1, s2, n) strncmp ((s1)->ds_string, (s2)->ds_string, n)
+-#define ds_index(s, c) index ((s)->ds_string, c)
+-#define ds_rindex(s, c) rindex ((s)->ds_string, c)
++void ds_init (dynamic_string *string);
++void ds_free (dynamic_string *string);
++void ds_reset (dynamic_string *s, size_t len);
+
+-void ds_init (dynamic_string *string, int size);
+-void ds_resize (dynamic_string *string, int size);
++/* All functions below guarantee that s->ds_string[s->ds_idx] == '\0' */
+ char *ds_fgetname (FILE *f, dynamic_string *s);
+ char *ds_fgets (FILE *f, dynamic_string *s);
+ char *ds_fgetstr (FILE *f, dynamic_string *s, char eos);
++void ds_append (dynamic_string *s, int c);
++void ds_concat (dynamic_string *s, char const *str);
++
++#define ds_len(s) ((s)->ds_idx)
++
++int ds_endswith (dynamic_string *s, int c);
++
+diff --git a/src/util.c b/src/util.c
+index 4421b20..6d6bbaa 100644
+--- a/src/util.c
++++ b/src/util.c
+@@ -846,11 +846,9 @@ get_next_reel (int tape_des)
+ FILE *tty_out; /* File for interacting with user. */
+ int old_tape_des;
+ char *next_archive_name;
+- dynamic_string new_name;
++ dynamic_string new_name = DYNAMIC_STRING_INITIALIZER;
+ char *str_res;
+
+- ds_init (&new_name, 128);
+-
+ /* Open files for interactive communication. */
+ tty_in = fopen (TTY_NAME, "r");
+ if (tty_in == NULL)
+@@ -925,7 +923,7 @@ get_next_reel (int tape_des)
+ error (PAXEXIT_FAILURE, 0, _("internal error: tape descriptor changed from %d to %d"),
+ old_tape_des, tape_des);
+
+- free (new_name.ds_string);
++ ds_free (&new_name);
+ fclose (tty_in);
+ fclose (tty_out);
+ }
diff --git a/meta/recipes-extended/cpio/cpio_2.13.bb b/meta/recipes-extended/cpio/cpio_2.13.bb
index f4df826ed9..c65d6980f7 100644
--- a/meta/recipes-extended/cpio/cpio_2.13.bb
+++ b/meta/recipes-extended/cpio/cpio_2.13.bb
@@ -9,6 +9,7 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=f27defe1e96c2e1ecd4e0c9be8967949"
SRC_URI = "${GNU_MIRROR}/cpio/cpio-${PV}.tar.gz \
file://0001-Unset-need_charset_alias-when-building-for-musl.patch \
file://0002-src-global.c-Remove-superfluous-declaration-of-progr.patch \
+ file://0001-Rewrite-dynamic-string-support.patch \
"
SRC_URI[md5sum] = "389c5452d667c23b5eceb206f5000810"
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [OE-core][hardknott][PATCH] cpio: fix CVE-2021-38185
2021-09-13 4:49 [OE-core][hardknott][PATCH] cpio: fix CVE-2021-38185 Chen Qi
@ 2021-09-13 7:11 ` Anuj Mittal
2021-09-13 8:33 ` Chen Qi
0 siblings, 1 reply; 5+ messages in thread
From: Anuj Mittal @ 2021-09-13 7:11 UTC (permalink / raw)
To: Qi.Chen, openembedded-core
It looks like this has introduced regressions and we'll need at least
these two too:
https://git.savannah.gnu.org/cgit/cpio.git/commit/?id=dfc801c44a93bed7b3951905b188823d6a0432c8
https://git.savannah.gnu.org/cgit/cpio.git/commit/?id=236684f6deb3178043fe72a8e2faca538fa2aae1
Thanks,
Anuj
On Mon, 2021-09-13 at 12:49 +0800, Chen Qi wrote:
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
> .../0001-Rewrite-dynamic-string-support.patch | 458 ++++++++++++++++++
> meta/recipes-extended/cpio/cpio_2.13.bb | 1 +
> 2 files changed, 459 insertions(+)
> create mode 100644 meta/recipes-extended/cpio/cpio-2.13/0001-Rewrite-
> dynamic-string-support.patch
>
> diff --git a/meta/recipes-extended/cpio/cpio-2.13/0001-Rewrite-dynamic-
> string-support.patch b/meta/recipes-extended/cpio/cpio-2.13/0001-
> Rewrite-dynamic-string-support.patch
> new file mode 100644
> index 0000000000..93ed05184f
> --- /dev/null
> +++ b/meta/recipes-extended/cpio/cpio-2.13/0001-Rewrite-dynamic-string-
> support.patch
> @@ -0,0 +1,458 @@
> +From 0cdda8baddbbc50a4212e36a470053ac624075bb Mon Sep 17 00:00:00 2001
> +From: Sergey Poznyakoff <gray@gnu.org>
> +Date: Sat, 7 Aug 2021 12:52:21 +0300
> +Subject: [PATCH] Rewrite dynamic string support.
> +
> +* src/dstring.c (ds_init): Take a single argument.
> +(ds_free): New function.
> +(ds_resize): Take a single argument. Use x2nrealloc to expand
> +the storage.
> +(ds_reset,ds_append,ds_concat,ds_endswith): New function.
> +(ds_fgetstr): Rewrite. In particular, this fixes integer overflow.
> +* src/dstring.h (dynamic_string): Keep both the allocated length
> +(ds_size) and index of the next free byte in the string (ds_idx).
> +(ds_init,ds_resize): Change signature.
> +(ds_len): New macro.
> +(ds_free,ds_reset,ds_append,ds_concat,ds_endswith): New protos.
> +* src/copyin.c: Use new ds_ functions.
> +* src/copyout.c: Likewise.
> +* src/copypass.c: Likewise.
> +* src/util.c: Likewise.
> +
> +Upstream-Status: Backport
> +CVE: CVE-2021-38185
> +Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> +---
> + src/copyin.c | 40 +++++++++++------------
> + src/copyout.c | 16 ++++-----
> + src/copypass.c | 34 +++++++++----------
> + src/dstring.c | 88 ++++++++++++++++++++++++++++++++++++-------------
> -
> + src/dstring.h | 31 +++++++++---------
> + src/util.c | 6 ++--
> + 6 files changed, 123 insertions(+), 92 deletions(-)
> +
> +diff --git a/src/copyin.c b/src/copyin.c
> +index b29f348..37e503a 100644
> +--- a/src/copyin.c
> ++++ b/src/copyin.c
> +@@ -55,11 +55,12 @@ query_rename(struct cpio_file_stat* file_hdr, FILE
> *tty_in, FILE *tty_out,
> + char *str_res; /* Result for string function. */
> + static dynamic_string new_name; /* New file name for rename
> option. */
> + static int initialized_new_name = false;
> ++
> + if (!initialized_new_name)
> +- {
> +- ds_init (&new_name, 128);
> +- initialized_new_name = true;
> +- }
> ++ {
> ++ ds_init (&new_name);
> ++ initialized_new_name = true;
> ++ }
> +
> + if (rename_flag)
> + {
> +@@ -779,37 +780,36 @@ long_format (struct cpio_file_stat *file_hdr,
> char const *link_name)
> + already in `save_patterns' (from the command line) are preserved.
> */
> +
> + static void
> +-read_pattern_file ()
> ++read_pattern_file (void)
> + {
> +- int max_new_patterns;
> +- char **new_save_patterns;
> +- int new_num_patterns;
> ++ char **new_save_patterns = NULL;
> ++ size_t max_new_patterns;
> ++ size_t new_num_patterns;
> + int i;
> +- dynamic_string pattern_name;
> ++ dynamic_string pattern_name = DYNAMIC_STRING_INITIALIZER;
> + FILE *pattern_fp;
> +
> + if (num_patterns < 0)
> + num_patterns = 0;
> +- max_new_patterns = 1 + num_patterns;
> +- new_save_patterns = (char **) xmalloc (max_new_patterns * sizeof
> (char *));
> + new_num_patterns = num_patterns;
> +- ds_init (&pattern_name, 128);
> ++ max_new_patterns = num_patterns;
> ++ new_save_patterns = xcalloc (max_new_patterns, sizeof
> (new_save_patterns[0]));
> +
> + pattern_fp = fopen (pattern_file_name, "r");
> + if (pattern_fp == NULL)
> + open_fatal (pattern_file_name);
> + while (ds_fgetstr (pattern_fp, &pattern_name, '\n') != NULL)
> + {
> +- if (new_num_patterns >= max_new_patterns)
> +- {
> +- max_new_patterns += 1;
> +- new_save_patterns = (char **)
> +- xrealloc ((char *) new_save_patterns,
> +- max_new_patterns * sizeof (char *));
> +- }
> ++ if (new_num_patterns == max_new_patterns)
> ++ new_save_patterns = x2nrealloc (new_save_patterns,
> ++ &max_new_patterns,
> ++ sizeof (new_save_patterns[0]));
> + new_save_patterns[new_num_patterns] = xstrdup
> (pattern_name.ds_string);
> + ++new_num_patterns;
> + }
> ++
> ++ ds_free (&pattern_name);
> ++
> + if (ferror (pattern_fp) || fclose (pattern_fp) == EOF)
> + close_error (pattern_file_name);
> +
> +@@ -1196,7 +1196,7 @@ swab_array (char *ptr, int count)
> + in the file system. */
> +
> + void
> +-process_copy_in ()
> ++process_copy_in (void)
> + {
> + char done = false; /* True if trailer reached. */
> + FILE *tty_in = NULL; /* Interactive file for rename
> option. */
> +diff --git a/src/copyout.c b/src/copyout.c
> +index 8b0beb6..26e3dda 100644
> +--- a/src/copyout.c
> ++++ b/src/copyout.c
> +@@ -594,9 +594,10 @@ assign_string (char **pvar, char *value)
> + The format of the header depends on the compatibility (-c) flag.
> */
> +
> + void
> +-process_copy_out ()
> ++process_copy_out (void)
> + {
> +- dynamic_string input_name; /* Name of file read from stdin. */
> ++ dynamic_string input_name = DYNAMIC_STRING_INITIALIZER;
> ++ /* Name of file read from stdin. */
> + struct stat file_stat; /* Stat record for file. */
> + struct cpio_file_stat file_hdr = CPIO_FILE_STAT_INITIALIZER;
> + /* Output header information. */
> +@@ -605,7 +606,6 @@ process_copy_out ()
> + char *orig_file_name = NULL;
> +
> + /* Initialize the copy out. */
> +- ds_init (&input_name, 128);
> + file_hdr.c_magic = 070707;
> +
> + /* Check whether the output file might be a tape. */
> +@@ -657,14 +657,9 @@ process_copy_out ()
> + {
> + if (file_hdr.c_mode & CP_IFDIR)
> + {
> +- int len = strlen (input_name.ds_string);
> + /* Make sure the name ends with a slash */
> +- if (input_name.ds_string[len-1] != '/')
> +- {
> +- ds_resize (&input_name, len + 2);
> +- input_name.ds_string[len] = '/';
> +- input_name.ds_string[len+1] = 0;
> +- }
> ++ if (!ds_endswith (&input_name, '/'))
> ++ ds_append (&input_name, '/');
> + }
> + }
> +
> +@@ -875,6 +870,7 @@ process_copy_out ()
> + (unsigned long) blocks), (unsigned long)
> blocks);
> + }
> + cpio_file_stat_free (&file_hdr);
> ++ ds_free (&input_name);
> + }
> +
> +
> +diff --git a/src/copypass.c b/src/copypass.c
> +index dc13b5b..62f31c6 100644
> +--- a/src/copypass.c
> ++++ b/src/copypass.c
> +@@ -48,10 +48,12 @@ set_copypass_perms (int fd, const char *name,
> struct stat *st)
> + If `link_flag', link instead of copying. */
> +
> + void
> +-process_copy_pass ()
> ++process_copy_pass (void)
> + {
> +- dynamic_string input_name; /* Name of file from stdin. */
> +- dynamic_string output_name; /* Name of new file. */
> ++ dynamic_string input_name = DYNAMIC_STRING_INITIALIZER;
> ++ /* Name of file from stdin. */
> ++ dynamic_string output_name = DYNAMIC_STRING_INITIALIZER;
> ++ /* Name of new file. */
> + size_t dirname_len; /* Length of `directory_name'. */
> + int res; /* Result of functions. */
> + char *slash; /* For moving past slashes in
> input name. */
> +@@ -65,25 +67,18 @@ process_copy_pass ()
> + created files */
> +
> + /* Initialize the copy pass. */
> +- ds_init (&input_name, 128);
> +
> + dirname_len = strlen (directory_name);
> + if (change_directory_option && !ISSLASH (directory_name[0]))
> + {
> + char *pwd = xgetcwd ();
> +-
> +- dirname_len += strlen (pwd) + 1;
> +- ds_init (&output_name, dirname_len + 2);
> +- strcpy (output_name.ds_string, pwd);
> +- strcat (output_name.ds_string, "/");
> +- strcat (output_name.ds_string, directory_name);
> ++
> ++ ds_concat (&output_name, pwd);
> ++ ds_append (&output_name, '/');
> + }
> +- else
> +- {
> +- ds_init (&output_name, dirname_len + 2);
> +- strcpy (output_name.ds_string, directory_name);
> +- }
> +- output_name.ds_string[dirname_len] = '/';
> ++ ds_concat (&output_name, directory_name);
> ++ ds_append (&output_name, '/');
> ++ dirname_len = ds_len (&output_name);
> + output_is_seekable = true;
> +
> + change_dir ();
> +@@ -116,8 +111,8 @@ process_copy_pass ()
> + /* Make the name of the new file. */
> + for (slash = input_name.ds_string; *slash == '/'; ++slash)
> + ;
> +- ds_resize (&output_name, dirname_len + strlen (slash) + 2);
> +- strcpy (output_name.ds_string + dirname_len + 1, slash);
> ++ ds_reset (&output_name, dirname_len);
> ++ ds_concat (&output_name, slash);
> +
> + existing_dir = false;
> + if (lstat (output_name.ds_string, &out_file_stat) == 0)
> +@@ -333,6 +328,9 @@ process_copy_pass ()
> + (unsigned long) blocks),
> + (unsigned long) blocks);
> + }
> ++
> ++ ds_free (&input_name);
> ++ ds_free (&output_name);
> + }
> + \f
> + /* Try and create a hard link from FILE_NAME to another file
> +diff --git a/src/dstring.c b/src/dstring.c
> +index e9c063f..358f356 100644
> +--- a/src/dstring.c
> ++++ b/src/dstring.c
> +@@ -20,8 +20,8 @@
> + #if defined(HAVE_CONFIG_H)
> + # include <config.h>
> + #endif
> +-
> + #include <stdio.h>
> ++#include <stdlib.h>
> + #if defined(HAVE_STRING_H) || defined(STDC_HEADERS)
> + #include <string.h>
> + #else
> +@@ -33,24 +33,41 @@
> + /* Initialiaze dynamic string STRING with space for SIZE characters.
> */
> +
> + void
> +-ds_init (dynamic_string *string, int size)
> ++ds_init (dynamic_string *string)
> ++{
> ++ memset (string, 0, sizeof *string);
> ++}
> ++
> ++/* Free the dynamic string storage. */
> ++
> ++void
> ++ds_free (dynamic_string *string)
> + {
> +- string->ds_length = size;
> +- string->ds_string = (char *) xmalloc (size);
> ++ free (string->ds_string);
> + }
> +
> +-/* Expand dynamic string STRING, if necessary, to hold SIZE
> characters. */
> ++/* Expand dynamic string STRING, if necessary. */
> +
> + void
> +-ds_resize (dynamic_string *string, int size)
> ++ds_resize (dynamic_string *string)
> + {
> +- if (size > string->ds_length)
> ++ if (string->ds_idx == string->ds_size)
> + {
> +- string->ds_length = size;
> +- string->ds_string = (char *) xrealloc ((char *) string-
> >ds_string, size);
> ++ string->ds_string = x2nrealloc (string->ds_string, &string-
> >ds_size,
> ++ 1);
> + }
> + }
> +
> ++/* Reset the index of the dynamic string S to LEN. */
> ++
> ++void
> ++ds_reset (dynamic_string *s, size_t len)
> ++{
> ++ while (len > s->ds_size)
> ++ ds_resize (s);
> ++ s->ds_idx = len;
> ++}
> ++
> + /* Dynamic string S gets a string terminated by the EOS character
> + (which is removed) from file F. S will increase
> + in size during the function if the string from F is longer than
> +@@ -61,34 +78,50 @@ ds_resize (dynamic_string *string, int size)
> + char *
> + ds_fgetstr (FILE *f, dynamic_string *s, char eos)
> + {
> +- int insize; /* Amount needed for line. */
> +- int strsize; /* Amount allocated for S. */
> + int next_ch;
> +
> + /* Initialize. */
> +- insize = 0;
> +- strsize = s->ds_length;
> ++ s->ds_idx = 0;
> +
> + /* Read the input string. */
> +- next_ch = getc (f);
> +- while (next_ch != eos && next_ch != EOF)
> ++ while ((next_ch = getc (f)) != eos && next_ch != EOF)
> + {
> +- if (insize >= strsize - 1)
> +- {
> +- ds_resize (s, strsize * 2 + 2);
> +- strsize = s->ds_length;
> +- }
> +- s->ds_string[insize++] = next_ch;
> +- next_ch = getc (f);
> ++ ds_resize (s);
> ++ s->ds_string[s->ds_idx++] = next_ch;
> + }
> +- s->ds_string[insize++] = '\0';
> ++ ds_resize (s);
> ++ s->ds_string[s->ds_idx] = '\0';
> +
> +- if (insize == 1 && next_ch == EOF)
> ++ if (s->ds_idx == 0 && next_ch == EOF)
> + return NULL;
> + else
> + return s->ds_string;
> + }
> +
> ++void
> ++ds_append (dynamic_string *s, int c)
> ++{
> ++ ds_resize (s);
> ++ s->ds_string[s->ds_idx] = c;
> ++ if (c)
> ++ {
> ++ s->ds_idx++;
> ++ ds_resize (s);
> ++ s->ds_string[s->ds_idx] = 0;
> ++ }
> ++}
> ++
> ++void
> ++ds_concat (dynamic_string *s, char const *str)
> ++{
> ++ size_t len = strlen (str);
> ++ while (len + 1 > s->ds_size)
> ++ ds_resize (s);
> ++ memcpy (s->ds_string + s->ds_idx, str, len);
> ++ s->ds_idx += len;
> ++ s->ds_string[s->ds_idx] = 0;
> ++}
> ++
> + char *
> + ds_fgets (FILE *f, dynamic_string *s)
> + {
> +@@ -100,3 +133,10 @@ ds_fgetname (FILE *f, dynamic_string *s)
> + {
> + return ds_fgetstr (f, s, '\0');
> + }
> ++
> ++/* Return true if the dynamic string S ends with character C. */
> ++int
> ++ds_endswith (dynamic_string *s, int c)
> ++{
> ++ return (s->ds_idx > 0 && s->ds_string[s->ds_idx - 1] == c);
> ++}
> +diff --git a/src/dstring.h b/src/dstring.h
> +index b5135fe..f5b04ef 100644
> +--- a/src/dstring.h
> ++++ b/src/dstring.h
> +@@ -17,10 +17,6 @@
> + Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> + Boston, MA 02110-1301 USA. */
> +
> +-#ifndef NULL
> +-#define NULL 0
> +-#endif
> +-
> + /* A dynamic string consists of record that records the size of an
> + allocated string and the pointer to that string. The actual
> string
> + is a normal zero byte terminated string that can be used with the
> +@@ -30,22 +26,25 @@
> +
> + typedef struct
> + {
> +- int ds_length; /* Actual amount of storage allocated.
> */
> +- char *ds_string; /* String. */
> ++ size_t ds_size; /* Actual amount of storage allocated. */
> ++ size_t ds_idx; /* Index of the next free byte in the string. */
> ++ char *ds_string; /* String storage. */
> + } dynamic_string;
> +
> ++#define DYNAMIC_STRING_INITIALIZER { 0, 0, NULL }
> +
> +-/* Macros that look similar to the original string functions.
> +- WARNING: These macros work only on pointers to dynamic string
> records.
> +- If used with a real record, an "&" must be used to get the
> pointer. */
> +-#define ds_strlen(s) strlen ((s)->ds_string)
> +-#define ds_strcmp(s1, s2) strcmp ((s1)->ds_string, (s2)-
> >ds_string)
> +-#define ds_strncmp(s1, s2, n) strncmp ((s1)->ds_string, (s2)-
> >ds_string, n)
> +-#define ds_index(s, c) index ((s)->ds_string, c)
> +-#define ds_rindex(s, c) rindex ((s)->ds_string, c)
> ++void ds_init (dynamic_string *string);
> ++void ds_free (dynamic_string *string);
> ++void ds_reset (dynamic_string *s, size_t len);
> +
> +-void ds_init (dynamic_string *string, int size);
> +-void ds_resize (dynamic_string *string, int size);
> ++/* All functions below guarantee that s->ds_string[s->ds_idx] == '\0'
> */
> + char *ds_fgetname (FILE *f, dynamic_string *s);
> + char *ds_fgets (FILE *f, dynamic_string *s);
> + char *ds_fgetstr (FILE *f, dynamic_string *s, char eos);
> ++void ds_append (dynamic_string *s, int c);
> ++void ds_concat (dynamic_string *s, char const *str);
> ++
> ++#define ds_len(s) ((s)->ds_idx)
> ++
> ++int ds_endswith (dynamic_string *s, int c);
> ++
> +diff --git a/src/util.c b/src/util.c
> +index 4421b20..6d6bbaa 100644
> +--- a/src/util.c
> ++++ b/src/util.c
> +@@ -846,11 +846,9 @@ get_next_reel (int tape_des)
> + FILE *tty_out; /* File for interacting with user. */
> + int old_tape_des;
> + char *next_archive_name;
> +- dynamic_string new_name;
> ++ dynamic_string new_name = DYNAMIC_STRING_INITIALIZER;
> + char *str_res;
> +
> +- ds_init (&new_name, 128);
> +-
> + /* Open files for interactive communication. */
> + tty_in = fopen (TTY_NAME, "r");
> + if (tty_in == NULL)
> +@@ -925,7 +923,7 @@ get_next_reel (int tape_des)
> + error (PAXEXIT_FAILURE, 0, _("internal error: tape descriptor
> changed from %d to %d"),
> + old_tape_des, tape_des);
> +
> +- free (new_name.ds_string);
> ++ ds_free (&new_name);
> + fclose (tty_in);
> + fclose (tty_out);
> + }
> diff --git a/meta/recipes-extended/cpio/cpio_2.13.bb b/meta/recipes-
> extended/cpio/cpio_2.13.bb
> index f4df826ed9..c65d6980f7 100644
> --- a/meta/recipes-extended/cpio/cpio_2.13.bb
> +++ b/meta/recipes-extended/cpio/cpio_2.13.bb
> @@ -9,6 +9,7 @@ LIC_FILES_CHKSUM =
> "file://COPYING;md5=f27defe1e96c2e1ecd4e0c9be8967949"
> SRC_URI = "${GNU_MIRROR}/cpio/cpio-${PV}.tar.gz \
>
> file://0001-Unset-need_charset_alias-when-building-for-musl.patch \
>
> file://0002-src-global.c-Remove-superfluous-declaration-of-progr.patch
> \
> + file://0001-Rewrite-dynamic-string-support.patch \
> "
>
> SRC_URI[md5sum] = "389c5452d667c23b5eceb206f5000810"
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [OE-core][hardknott][PATCH] cpio: fix CVE-2021-38185
2021-09-13 7:11 ` Anuj Mittal
@ 2021-09-13 8:33 ` Chen Qi
2021-09-13 10:55 ` Richard Purdie
0 siblings, 1 reply; 5+ messages in thread
From: Chen Qi @ 2021-09-13 8:33 UTC (permalink / raw)
To: Mittal, Anuj, openembedded-core
Just found that Ross has sent out a patch for CVE-2021-38185 and it has
been merged in hardknott.
So please ignore this patch.
I'm also curious about how you spot such issue. By double checking the
commit logs in cpio repo?
Best Regards,
Chen Qi
On 09/13/2021 03:11 PM, Mittal, Anuj wrote:
> It looks like this has introduced regressions and we'll need at least
> these two too:
>
> https://git.savannah.gnu.org/cgit/cpio.git/commit/?id=dfc801c44a93bed7b3951905b188823d6a0432c8
> https://git.savannah.gnu.org/cgit/cpio.git/commit/?id=236684f6deb3178043fe72a8e2faca538fa2aae1
>
> Thanks,
>
> Anuj
>
> On Mon, 2021-09-13 at 12:49 +0800, Chen Qi wrote:
>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>> ---
>> .../0001-Rewrite-dynamic-string-support.patch | 458 ++++++++++++++++++
>> meta/recipes-extended/cpio/cpio_2.13.bb | 1 +
>> 2 files changed, 459 insertions(+)
>> create mode 100644 meta/recipes-extended/cpio/cpio-2.13/0001-Rewrite-
>> dynamic-string-support.patch
>>
>> diff --git a/meta/recipes-extended/cpio/cpio-2.13/0001-Rewrite-dynamic-
>> string-support.patch b/meta/recipes-extended/cpio/cpio-2.13/0001-
>> Rewrite-dynamic-string-support.patch
>> new file mode 100644
>> index 0000000000..93ed05184f
>> --- /dev/null
>> +++ b/meta/recipes-extended/cpio/cpio-2.13/0001-Rewrite-dynamic-string-
>> support.patch
>> @@ -0,0 +1,458 @@
>> +From 0cdda8baddbbc50a4212e36a470053ac624075bb Mon Sep 17 00:00:00 2001
>> +From: Sergey Poznyakoff <gray@gnu.org>
>> +Date: Sat, 7 Aug 2021 12:52:21 +0300
>> +Subject: [PATCH] Rewrite dynamic string support.
>> +
>> +* src/dstring.c (ds_init): Take a single argument.
>> +(ds_free): New function.
>> +(ds_resize): Take a single argument. Use x2nrealloc to expand
>> +the storage.
>> +(ds_reset,ds_append,ds_concat,ds_endswith): New function.
>> +(ds_fgetstr): Rewrite. In particular, this fixes integer overflow.
>> +* src/dstring.h (dynamic_string): Keep both the allocated length
>> +(ds_size) and index of the next free byte in the string (ds_idx).
>> +(ds_init,ds_resize): Change signature.
>> +(ds_len): New macro.
>> +(ds_free,ds_reset,ds_append,ds_concat,ds_endswith): New protos.
>> +* src/copyin.c: Use new ds_ functions.
>> +* src/copyout.c: Likewise.
>> +* src/copypass.c: Likewise.
>> +* src/util.c: Likewise.
>> +
>> +Upstream-Status: Backport
>> +CVE: CVE-2021-38185
>> +Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>> +---
>> + src/copyin.c | 40 +++++++++++------------
>> + src/copyout.c | 16 ++++-----
>> + src/copypass.c | 34 +++++++++----------
>> + src/dstring.c | 88 ++++++++++++++++++++++++++++++++++++-------------
>> -
>> + src/dstring.h | 31 +++++++++---------
>> + src/util.c | 6 ++--
>> + 6 files changed, 123 insertions(+), 92 deletions(-)
>> +
>> +diff --git a/src/copyin.c b/src/copyin.c
>> +index b29f348..37e503a 100644
>> +--- a/src/copyin.c
>> ++++ b/src/copyin.c
>> +@@ -55,11 +55,12 @@ query_rename(struct cpio_file_stat* file_hdr, FILE
>> *tty_in, FILE *tty_out,
>> + char *str_res; /* Result for string function. */
>> + static dynamic_string new_name; /* New file name for rename
>> option. */
>> + static int initialized_new_name = false;
>> ++
>> + if (!initialized_new_name)
>> +- {
>> +- ds_init (&new_name, 128);
>> +- initialized_new_name = true;
>> +- }
>> ++ {
>> ++ ds_init (&new_name);
>> ++ initialized_new_name = true;
>> ++ }
>> +
>> + if (rename_flag)
>> + {
>> +@@ -779,37 +780,36 @@ long_format (struct cpio_file_stat *file_hdr,
>> char const *link_name)
>> + already in `save_patterns' (from the command line) are preserved.
>> */
>> +
>> + static void
>> +-read_pattern_file ()
>> ++read_pattern_file (void)
>> + {
>> +- int max_new_patterns;
>> +- char **new_save_patterns;
>> +- int new_num_patterns;
>> ++ char **new_save_patterns = NULL;
>> ++ size_t max_new_patterns;
>> ++ size_t new_num_patterns;
>> + int i;
>> +- dynamic_string pattern_name;
>> ++ dynamic_string pattern_name = DYNAMIC_STRING_INITIALIZER;
>> + FILE *pattern_fp;
>> +
>> + if (num_patterns < 0)
>> + num_patterns = 0;
>> +- max_new_patterns = 1 + num_patterns;
>> +- new_save_patterns = (char **) xmalloc (max_new_patterns * sizeof
>> (char *));
>> + new_num_patterns = num_patterns;
>> +- ds_init (&pattern_name, 128);
>> ++ max_new_patterns = num_patterns;
>> ++ new_save_patterns = xcalloc (max_new_patterns, sizeof
>> (new_save_patterns[0]));
>> +
>> + pattern_fp = fopen (pattern_file_name, "r");
>> + if (pattern_fp == NULL)
>> + open_fatal (pattern_file_name);
>> + while (ds_fgetstr (pattern_fp, &pattern_name, '\n') != NULL)
>> + {
>> +- if (new_num_patterns >= max_new_patterns)
>> +- {
>> +- max_new_patterns += 1;
>> +- new_save_patterns = (char **)
>> +- xrealloc ((char *) new_save_patterns,
>> +- max_new_patterns * sizeof (char *));
>> +- }
>> ++ if (new_num_patterns == max_new_patterns)
>> ++ new_save_patterns = x2nrealloc (new_save_patterns,
>> ++ &max_new_patterns,
>> ++ sizeof (new_save_patterns[0]));
>> + new_save_patterns[new_num_patterns] = xstrdup
>> (pattern_name.ds_string);
>> + ++new_num_patterns;
>> + }
>> ++
>> ++ ds_free (&pattern_name);
>> ++
>> + if (ferror (pattern_fp) || fclose (pattern_fp) == EOF)
>> + close_error (pattern_file_name);
>> +
>> +@@ -1196,7 +1196,7 @@ swab_array (char *ptr, int count)
>> + in the file system. */
>> +
>> + void
>> +-process_copy_in ()
>> ++process_copy_in (void)
>> + {
>> + char done = false; /* True if trailer reached. */
>> + FILE *tty_in = NULL; /* Interactive file for rename
>> option. */
>> +diff --git a/src/copyout.c b/src/copyout.c
>> +index 8b0beb6..26e3dda 100644
>> +--- a/src/copyout.c
>> ++++ b/src/copyout.c
>> +@@ -594,9 +594,10 @@ assign_string (char **pvar, char *value)
>> + The format of the header depends on the compatibility (-c) flag.
>> */
>> +
>> + void
>> +-process_copy_out ()
>> ++process_copy_out (void)
>> + {
>> +- dynamic_string input_name; /* Name of file read from stdin. */
>> ++ dynamic_string input_name = DYNAMIC_STRING_INITIALIZER;
>> ++ /* Name of file read from stdin. */
>> + struct stat file_stat; /* Stat record for file. */
>> + struct cpio_file_stat file_hdr = CPIO_FILE_STAT_INITIALIZER;
>> + /* Output header information. */
>> +@@ -605,7 +606,6 @@ process_copy_out ()
>> + char *orig_file_name = NULL;
>> +
>> + /* Initialize the copy out. */
>> +- ds_init (&input_name, 128);
>> + file_hdr.c_magic = 070707;
>> +
>> + /* Check whether the output file might be a tape. */
>> +@@ -657,14 +657,9 @@ process_copy_out ()
>> + {
>> + if (file_hdr.c_mode & CP_IFDIR)
>> + {
>> +- int len = strlen (input_name.ds_string);
>> + /* Make sure the name ends with a slash */
>> +- if (input_name.ds_string[len-1] != '/')
>> +- {
>> +- ds_resize (&input_name, len + 2);
>> +- input_name.ds_string[len] = '/';
>> +- input_name.ds_string[len+1] = 0;
>> +- }
>> ++ if (!ds_endswith (&input_name, '/'))
>> ++ ds_append (&input_name, '/');
>> + }
>> + }
>> +
>> +@@ -875,6 +870,7 @@ process_copy_out ()
>> + (unsigned long) blocks), (unsigned long)
>> blocks);
>> + }
>> + cpio_file_stat_free (&file_hdr);
>> ++ ds_free (&input_name);
>> + }
>> +
>> +
>> +diff --git a/src/copypass.c b/src/copypass.c
>> +index dc13b5b..62f31c6 100644
>> +--- a/src/copypass.c
>> ++++ b/src/copypass.c
>> +@@ -48,10 +48,12 @@ set_copypass_perms (int fd, const char *name,
>> struct stat *st)
>> + If `link_flag', link instead of copying. */
>> +
>> + void
>> +-process_copy_pass ()
>> ++process_copy_pass (void)
>> + {
>> +- dynamic_string input_name; /* Name of file from stdin. */
>> +- dynamic_string output_name; /* Name of new file. */
>> ++ dynamic_string input_name = DYNAMIC_STRING_INITIALIZER;
>> ++ /* Name of file from stdin. */
>> ++ dynamic_string output_name = DYNAMIC_STRING_INITIALIZER;
>> ++ /* Name of new file. */
>> + size_t dirname_len; /* Length of `directory_name'. */
>> + int res; /* Result of functions. */
>> + char *slash; /* For moving past slashes in
>> input name. */
>> +@@ -65,25 +67,18 @@ process_copy_pass ()
>> + created files */
>> +
>> + /* Initialize the copy pass. */
>> +- ds_init (&input_name, 128);
>> +
>> + dirname_len = strlen (directory_name);
>> + if (change_directory_option && !ISSLASH (directory_name[0]))
>> + {
>> + char *pwd = xgetcwd ();
>> +-
>> +- dirname_len += strlen (pwd) + 1;
>> +- ds_init (&output_name, dirname_len + 2);
>> +- strcpy (output_name.ds_string, pwd);
>> +- strcat (output_name.ds_string, "/");
>> +- strcat (output_name.ds_string, directory_name);
>> ++
>> ++ ds_concat (&output_name, pwd);
>> ++ ds_append (&output_name, '/');
>> + }
>> +- else
>> +- {
>> +- ds_init (&output_name, dirname_len + 2);
>> +- strcpy (output_name.ds_string, directory_name);
>> +- }
>> +- output_name.ds_string[dirname_len] = '/';
>> ++ ds_concat (&output_name, directory_name);
>> ++ ds_append (&output_name, '/');
>> ++ dirname_len = ds_len (&output_name);
>> + output_is_seekable = true;
>> +
>> + change_dir ();
>> +@@ -116,8 +111,8 @@ process_copy_pass ()
>> + /* Make the name of the new file. */
>> + for (slash = input_name.ds_string; *slash == '/'; ++slash)
>> + ;
>> +- ds_resize (&output_name, dirname_len + strlen (slash) + 2);
>> +- strcpy (output_name.ds_string + dirname_len + 1, slash);
>> ++ ds_reset (&output_name, dirname_len);
>> ++ ds_concat (&output_name, slash);
>> +
>> + existing_dir = false;
>> + if (lstat (output_name.ds_string, &out_file_stat) == 0)
>> +@@ -333,6 +328,9 @@ process_copy_pass ()
>> + (unsigned long) blocks),
>> + (unsigned long) blocks);
>> + }
>> ++
>> ++ ds_free (&input_name);
>> ++ ds_free (&output_name);
>> + }
>> + \f
>> + /* Try and create a hard link from FILE_NAME to another file
>> +diff --git a/src/dstring.c b/src/dstring.c
>> +index e9c063f..358f356 100644
>> +--- a/src/dstring.c
>> ++++ b/src/dstring.c
>> +@@ -20,8 +20,8 @@
>> + #if defined(HAVE_CONFIG_H)
>> + # include <config.h>
>> + #endif
>> +-
>> + #include <stdio.h>
>> ++#include <stdlib.h>
>> + #if defined(HAVE_STRING_H) || defined(STDC_HEADERS)
>> + #include <string.h>
>> + #else
>> +@@ -33,24 +33,41 @@
>> + /* Initialiaze dynamic string STRING with space for SIZE characters.
>> */
>> +
>> + void
>> +-ds_init (dynamic_string *string, int size)
>> ++ds_init (dynamic_string *string)
>> ++{
>> ++ memset (string, 0, sizeof *string);
>> ++}
>> ++
>> ++/* Free the dynamic string storage. */
>> ++
>> ++void
>> ++ds_free (dynamic_string *string)
>> + {
>> +- string->ds_length = size;
>> +- string->ds_string = (char *) xmalloc (size);
>> ++ free (string->ds_string);
>> + }
>> +
>> +-/* Expand dynamic string STRING, if necessary, to hold SIZE
>> characters. */
>> ++/* Expand dynamic string STRING, if necessary. */
>> +
>> + void
>> +-ds_resize (dynamic_string *string, int size)
>> ++ds_resize (dynamic_string *string)
>> + {
>> +- if (size > string->ds_length)
>> ++ if (string->ds_idx == string->ds_size)
>> + {
>> +- string->ds_length = size;
>> +- string->ds_string = (char *) xrealloc ((char *) string-
>>> ds_string, size);
>> ++ string->ds_string = x2nrealloc (string->ds_string, &string-
>>> ds_size,
>> ++ 1);
>> + }
>> + }
>> +
>> ++/* Reset the index of the dynamic string S to LEN. */
>> ++
>> ++void
>> ++ds_reset (dynamic_string *s, size_t len)
>> ++{
>> ++ while (len > s->ds_size)
>> ++ ds_resize (s);
>> ++ s->ds_idx = len;
>> ++}
>> ++
>> + /* Dynamic string S gets a string terminated by the EOS character
>> + (which is removed) from file F. S will increase
>> + in size during the function if the string from F is longer than
>> +@@ -61,34 +78,50 @@ ds_resize (dynamic_string *string, int size)
>> + char *
>> + ds_fgetstr (FILE *f, dynamic_string *s, char eos)
>> + {
>> +- int insize; /* Amount needed for line. */
>> +- int strsize; /* Amount allocated for S. */
>> + int next_ch;
>> +
>> + /* Initialize. */
>> +- insize = 0;
>> +- strsize = s->ds_length;
>> ++ s->ds_idx = 0;
>> +
>> + /* Read the input string. */
>> +- next_ch = getc (f);
>> +- while (next_ch != eos && next_ch != EOF)
>> ++ while ((next_ch = getc (f)) != eos && next_ch != EOF)
>> + {
>> +- if (insize >= strsize - 1)
>> +- {
>> +- ds_resize (s, strsize * 2 + 2);
>> +- strsize = s->ds_length;
>> +- }
>> +- s->ds_string[insize++] = next_ch;
>> +- next_ch = getc (f);
>> ++ ds_resize (s);
>> ++ s->ds_string[s->ds_idx++] = next_ch;
>> + }
>> +- s->ds_string[insize++] = '\0';
>> ++ ds_resize (s);
>> ++ s->ds_string[s->ds_idx] = '\0';
>> +
>> +- if (insize == 1 && next_ch == EOF)
>> ++ if (s->ds_idx == 0 && next_ch == EOF)
>> + return NULL;
>> + else
>> + return s->ds_string;
>> + }
>> +
>> ++void
>> ++ds_append (dynamic_string *s, int c)
>> ++{
>> ++ ds_resize (s);
>> ++ s->ds_string[s->ds_idx] = c;
>> ++ if (c)
>> ++ {
>> ++ s->ds_idx++;
>> ++ ds_resize (s);
>> ++ s->ds_string[s->ds_idx] = 0;
>> ++ }
>> ++}
>> ++
>> ++void
>> ++ds_concat (dynamic_string *s, char const *str)
>> ++{
>> ++ size_t len = strlen (str);
>> ++ while (len + 1 > s->ds_size)
>> ++ ds_resize (s);
>> ++ memcpy (s->ds_string + s->ds_idx, str, len);
>> ++ s->ds_idx += len;
>> ++ s->ds_string[s->ds_idx] = 0;
>> ++}
>> ++
>> + char *
>> + ds_fgets (FILE *f, dynamic_string *s)
>> + {
>> +@@ -100,3 +133,10 @@ ds_fgetname (FILE *f, dynamic_string *s)
>> + {
>> + return ds_fgetstr (f, s, '\0');
>> + }
>> ++
>> ++/* Return true if the dynamic string S ends with character C. */
>> ++int
>> ++ds_endswith (dynamic_string *s, int c)
>> ++{
>> ++ return (s->ds_idx > 0 && s->ds_string[s->ds_idx - 1] == c);
>> ++}
>> +diff --git a/src/dstring.h b/src/dstring.h
>> +index b5135fe..f5b04ef 100644
>> +--- a/src/dstring.h
>> ++++ b/src/dstring.h
>> +@@ -17,10 +17,6 @@
>> + Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>> + Boston, MA 02110-1301 USA. */
>> +
>> +-#ifndef NULL
>> +-#define NULL 0
>> +-#endif
>> +-
>> + /* A dynamic string consists of record that records the size of an
>> + allocated string and the pointer to that string. The actual
>> string
>> + is a normal zero byte terminated string that can be used with the
>> +@@ -30,22 +26,25 @@
>> +
>> + typedef struct
>> + {
>> +- int ds_length; /* Actual amount of storage allocated.
>> */
>> +- char *ds_string; /* String. */
>> ++ size_t ds_size; /* Actual amount of storage allocated. */
>> ++ size_t ds_idx; /* Index of the next free byte in the string. */
>> ++ char *ds_string; /* String storage. */
>> + } dynamic_string;
>> +
>> ++#define DYNAMIC_STRING_INITIALIZER { 0, 0, NULL }
>> +
>> +-/* Macros that look similar to the original string functions.
>> +- WARNING: These macros work only on pointers to dynamic string
>> records.
>> +- If used with a real record, an "&" must be used to get the
>> pointer. */
>> +-#define ds_strlen(s) strlen ((s)->ds_string)
>> +-#define ds_strcmp(s1, s2) strcmp ((s1)->ds_string, (s2)-
>>> ds_string)
>> +-#define ds_strncmp(s1, s2, n) strncmp ((s1)->ds_string, (s2)-
>>> ds_string, n)
>> +-#define ds_index(s, c) index ((s)->ds_string, c)
>> +-#define ds_rindex(s, c) rindex ((s)->ds_string, c)
>> ++void ds_init (dynamic_string *string);
>> ++void ds_free (dynamic_string *string);
>> ++void ds_reset (dynamic_string *s, size_t len);
>> +
>> +-void ds_init (dynamic_string *string, int size);
>> +-void ds_resize (dynamic_string *string, int size);
>> ++/* All functions below guarantee that s->ds_string[s->ds_idx] == '\0'
>> */
>> + char *ds_fgetname (FILE *f, dynamic_string *s);
>> + char *ds_fgets (FILE *f, dynamic_string *s);
>> + char *ds_fgetstr (FILE *f, dynamic_string *s, char eos);
>> ++void ds_append (dynamic_string *s, int c);
>> ++void ds_concat (dynamic_string *s, char const *str);
>> ++
>> ++#define ds_len(s) ((s)->ds_idx)
>> ++
>> ++int ds_endswith (dynamic_string *s, int c);
>> ++
>> +diff --git a/src/util.c b/src/util.c
>> +index 4421b20..6d6bbaa 100644
>> +--- a/src/util.c
>> ++++ b/src/util.c
>> +@@ -846,11 +846,9 @@ get_next_reel (int tape_des)
>> + FILE *tty_out; /* File for interacting with user. */
>> + int old_tape_des;
>> + char *next_archive_name;
>> +- dynamic_string new_name;
>> ++ dynamic_string new_name = DYNAMIC_STRING_INITIALIZER;
>> + char *str_res;
>> +
>> +- ds_init (&new_name, 128);
>> +-
>> + /* Open files for interactive communication. */
>> + tty_in = fopen (TTY_NAME, "r");
>> + if (tty_in == NULL)
>> +@@ -925,7 +923,7 @@ get_next_reel (int tape_des)
>> + error (PAXEXIT_FAILURE, 0, _("internal error: tape descriptor
>> changed from %d to %d"),
>> + old_tape_des, tape_des);
>> +
>> +- free (new_name.ds_string);
>> ++ ds_free (&new_name);
>> + fclose (tty_in);
>> + fclose (tty_out);
>> + }
>> diff --git a/meta/recipes-extended/cpio/cpio_2.13.bb b/meta/recipes-
>> extended/cpio/cpio_2.13.bb
>> index f4df826ed9..c65d6980f7 100644
>> --- a/meta/recipes-extended/cpio/cpio_2.13.bb
>> +++ b/meta/recipes-extended/cpio/cpio_2.13.bb
>> @@ -9,6 +9,7 @@ LIC_FILES_CHKSUM =
>> "file://COPYING;md5=f27defe1e96c2e1ecd4e0c9be8967949"
>> SRC_URI = "${GNU_MIRROR}/cpio/cpio-${PV}.tar.gz \
>>
>> file://0001-Unset-need_charset_alias-when-building-for-musl.patch \
>>
>> file://0002-src-global.c-Remove-superfluous-declaration-of-progr.patch
>> \
>> + file://0001-Rewrite-dynamic-string-support.patch \
>> "
>>
>> SRC_URI[md5sum] = "389c5452d667c23b5eceb206f5000810"
>>
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [OE-core][hardknott][PATCH] cpio: fix CVE-2021-38185
2021-09-13 8:33 ` Chen Qi
@ 2021-09-13 10:55 ` Richard Purdie
2021-09-14 1:09 ` Anuj Mittal
0 siblings, 1 reply; 5+ messages in thread
From: Richard Purdie @ 2021-09-13 10:55 UTC (permalink / raw)
To: Chen Qi, Mittal, Anuj, openembedded-core
On Mon, 2021-09-13 at 16:33 +0800, Chen Qi wrote:
> Just found that Ross has sent out a patch for CVE-2021-38185 and it has
> been merged in hardknott.
> So please ignore this patch.
>
> I'm also curious about how you spot such issue. By double checking the
> commit logs in cpio repo?
I had looked at this one a little as well. I'd looked at some of the discussion
from the debian community on the CVE and noticed they talked about regressions,
which prompts you to look further into the issue.
Ross' patches in master included the regressions fixes too.
Cheers,
Richard
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [OE-core][hardknott][PATCH] cpio: fix CVE-2021-38185
2021-09-13 10:55 ` Richard Purdie
@ 2021-09-14 1:09 ` Anuj Mittal
0 siblings, 0 replies; 5+ messages in thread
From: Anuj Mittal @ 2021-09-14 1:09 UTC (permalink / raw)
To: Qi.Chen, richard.purdie, openembedded-core
On Mon, 2021-09-13 at 11:55 +0100, Richard Purdie wrote:
> On Mon, 2021-09-13 at 16:33 +0800, Chen Qi wrote:
> > Just found that Ross has sent out a patch for CVE-2021-38185 and it
> > has
> > been merged in hardknott.
> > So please ignore this patch.
> >
> > I'm also curious about how you spot such issue. By double checking
> > the
> > commit logs in cpio repo?
>
> I had looked at this one a little as well. I'd looked at some of the
> discussion
> from the debian community on the CVE and noticed they talked about
> regressions,
> which prompts you to look further into the issue.
>
Yeah, that's what I had done too. Debian tracker has a nice summary:
https://security-tracker.debian.org/tracker/CVE-2021-38185
Thanks,
Anuj
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-14 1:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13 4:49 [OE-core][hardknott][PATCH] cpio: fix CVE-2021-38185 Chen Qi
2021-09-13 7:11 ` Anuj Mittal
2021-09-13 8:33 ` Chen Qi
2021-09-13 10:55 ` Richard Purdie
2021-09-14 1:09 ` Anuj Mittal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.