summaryrefslogtreecommitdiffstats
path: root/meta/recipes-extended/cpio/cpio-2.12/0001-CVE-2016-2037-1-byte-out-of-bounds-write.patch
blob: 0a3054483cb8ea7fec1cea3fa7b1cfb265b785aa (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
From ebf9a2d776474181936a720ce811d72bbd1da3b6 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup <praiskup@redhat.com>
Date: Tue, 26 Jan 2016 23:17:54 +0100
Subject: [PATCH] CVE-2016-2037 - 1 byte out-of-bounds write

Ensure that cpio_safer_name_suffix always works with dynamically
allocated buffer, and that it has size of at least 32 bytes.
Then, any call to cpio_safer_name_suffix is safe (it requires at
least 2 bytes in the buffer).

Also ensure that c_namesize is always correctly initialized (by
cpio_set_c_name) to avoid undefined behavior when reading
file_hdr.c_namesize (previously happened for tar archives).

References:
http://www.mail-archive.com/bug-cpio@gnu.org/msg00545.html

* src/copyin.c (query_rename): Drop the hack, as we now work with
dynamically allocated buffer.  Use cpio_set_c_name.
(create_defered_links_to_skipped): Use cpio_set_c_name rather than
manual assignment.
(read_name_from_file): New function to avoid C&P.
(read_in_old_ascii, read_in_new_ascii, read_in_binary): Use
read_name_from_file.
(process_copy_in): Initialize file_hdr.c_namesize.
* src/copyout.c (process_copy_out): Use cpio_set_c_name.
* src/cpiohdr.h (cpio_set_c_name): New prototype.
* src/tar.c (read_in_tar_header): Use cpio_set_c_name.
* src/util.c (cpio_set_c_name): New function to set
file_hdr->c_name and c_namesize from arbitrary string.
(cpio_safer_name_suffix): Some docs fixes.
* tests/inout.at: Also test copy-in, and try various formats.

CVE: CVE-2016-2037

Upstream-Status: Backport [http://git.savannah.gnu.org/cgit/cpio.git/commit/?id=d36ec5f4e93130efb24fb9678aafd88e8070095b]

Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
---
 src/copyin.c   | 68 +++++++++++++++++++---------------------------------------
 src/copyout.c  | 13 +++++------
 src/cpiohdr.h  |  1 +
 src/tar.c      | 10 +++++----
 src/util.c     | 32 ++++++++++++++++++++++++++-
 tests/inout.at | 19 ++++++++++++++--
 6 files changed, 82 insertions(+), 61 deletions(-)

diff --git a/src/copyin.c b/src/copyin.c
index cde911e..972f8a6 100644
--- a/src/copyin.c
+++ b/src/copyin.c
@@ -76,28 +76,7 @@ query_rename(struct cpio_file_stat* file_hdr, FILE *tty_in, FILE *tty_out,
       return -1;
     }
   else
-  /* Debian hack: file_hrd.c_name is sometimes set to
-     point to static memory by code in tar.c.  This
-     causes a segfault.  This has been fixed and an
-     additional check to ensure that the file name
-     is not too long has been added.  (Reported by
-     Horst Knobloch.)  This bug has been reported to
-     "bug-gnu-utils@prep.ai.mit.edu". (99/1/6) -BEM */
-    {
-      if (archive_format != arf_tar && archive_format != arf_ustar)
-	{
-	  free (file_hdr->c_name);
-	  file_hdr->c_name = xstrdup (new_name.ds_string);
-	}
-      else
-	{
-	  if (is_tar_filename_too_long (new_name.ds_string))
-	    error (0, 0, _("%s: file name too long"),
-		   new_name.ds_string);
-	  else
-	    strcpy (file_hdr->c_name, new_name.ds_string);
-	}
-    }
+    cpio_set_c_name (file_hdr, new_name.ds_string);
   return 0;
 }
 
@@ -344,8 +323,7 @@ create_defered_links_to_skipped (struct cpio_file_stat *file_hdr,
 	    d_prev->next = d->next;
 	  else
 	    deferments = d->next;
-	  free (file_hdr->c_name);
-	  file_hdr->c_name = xstrdup(d->header.c_name);
+	  cpio_set_c_name (file_hdr, d->header.c_name);
 	  free_deferment (d);
 	  copyin_regular_file(file_hdr, in_file_des);
 	  return 0;
@@ -1064,6 +1042,22 @@ read_in_header (struct cpio_file_stat *file_hdr, int in_des)
     }
 }
 
+static void
+read_name_from_file (struct cpio_file_stat *file_hdr, int fd, uintmax_t len)
+{
+  static char *tmp_filename;
+  static size_t buflen;
+
+  if (buflen < len)
+    {
+      buflen = len;
+      tmp_filename = xrealloc (tmp_filename, buflen);
+    }
+
+  tape_buffered_read (tmp_filename, fd, len);
+  cpio_set_c_name (file_hdr, tmp_filename);
+}
+
 /* Fill in FILE_HDR by reading an old-format ASCII format cpio header from
    file descriptor IN_DES, except for the magic number, which is
    already filled in.  */
@@ -1090,14 +1084,8 @@ read_in_old_ascii (struct cpio_file_stat *file_hdr, int in_des)
   file_hdr->c_rdev_min = minor (dev);
 
   file_hdr->c_mtime = FROM_OCTAL (ascii_header.c_mtime);
-  file_hdr->c_namesize = FROM_OCTAL (ascii_header.c_namesize);
   file_hdr->c_filesize = FROM_OCTAL (ascii_header.c_filesize);
-  
-  /* Read file name from input.  */
-  if (file_hdr->c_name != NULL)
-    free (file_hdr->c_name);
-  file_hdr->c_name = (char *) xmalloc (file_hdr->c_namesize + 1);
-  tape_buffered_read (file_hdr->c_name, in_des, (long) file_hdr->c_namesize);
+  read_name_from_file (file_hdr, in_des, FROM_OCTAL (ascii_header.c_namesize));
 
   /* HP/UX cpio creates archives that look just like ordinary archives,
      but for devices it sets major = 0, minor = 1, and puts the
@@ -1152,14 +1140,8 @@ read_in_new_ascii (struct cpio_file_stat *file_hdr, int in_des)
   file_hdr->c_dev_min = FROM_HEX (ascii_header.c_dev_min);
   file_hdr->c_rdev_maj = FROM_HEX (ascii_header.c_rdev_maj);
   file_hdr->c_rdev_min = FROM_HEX (ascii_header.c_rdev_min);
-  file_hdr->c_namesize = FROM_HEX (ascii_header.c_namesize);
   file_hdr->c_chksum = FROM_HEX (ascii_header.c_chksum);
-  
-  /* Read file name from input.  */
-  if (file_hdr->c_name != NULL)
-    free (file_hdr->c_name);
-  file_hdr->c_name = (char *) xmalloc (file_hdr->c_namesize);
-  tape_buffered_read (file_hdr->c_name, in_des, (long) file_hdr->c_namesize);
+  read_name_from_file (file_hdr, in_des, FROM_HEX (ascii_header.c_namesize));
 
   /* In SVR4 ASCII format, the amount of space allocated for the header
      is rounded up to the next long-word, so we might need to drop
@@ -1207,16 +1189,9 @@ read_in_binary (struct cpio_file_stat *file_hdr,
   file_hdr->c_rdev_min = minor (short_hdr->c_rdev);
   file_hdr->c_mtime = (unsigned long) short_hdr->c_mtimes[0] << 16
                       | short_hdr->c_mtimes[1];
-
-  file_hdr->c_namesize = short_hdr->c_namesize;
   file_hdr->c_filesize = (unsigned long) short_hdr->c_filesizes[0] << 16
                       | short_hdr->c_filesizes[1];
-
-  /* Read file name from input.  */
-  if (file_hdr->c_name != NULL)
-    free (file_hdr->c_name);
-  file_hdr->c_name = (char *) xmalloc (file_hdr->c_namesize);
-  tape_buffered_read (file_hdr->c_name, in_des, (long) file_hdr->c_namesize);
+  read_name_from_file (file_hdr, in_des, short_hdr->c_namesize);
 
   /* In binary mode, the amount of space allocated in the header for
      the filename is `c_namesize' rounded up to the next short-word,
@@ -1297,6 +1272,7 @@ process_copy_in ()
       read_pattern_file ();
     }
   file_hdr.c_name = NULL;
+  file_hdr.c_namesize = 0;
 
   if (rename_batch_file)
     {
diff --git a/src/copyout.c b/src/copyout.c
index 1f0987a..bb39559 100644
--- a/src/copyout.c
+++ b/src/copyout.c
@@ -660,8 +660,7 @@ process_copy_out ()
 	  cpio_safer_name_suffix (input_name.ds_string, false,
 				  !no_abs_paths_flag, true);
 #ifndef HPUX_CDF
-	  file_hdr.c_name = input_name.ds_string;
-	  file_hdr.c_namesize = strlen (input_name.ds_string) + 1;
+	  cpio_set_c_name (&file_hdr, input_name.ds_string);
 #else
 	  if ( (archive_format != arf_tar) && (archive_format != arf_ustar) )
 	    {
@@ -670,16 +669,15 @@ process_copy_out ()
 		 properly recreate the directory as hidden (in case the
 		 files of a directory go into the archive before the
 		 directory itself (e.g from "find ... -depth ... | cpio")).  */
-	      file_hdr.c_name = add_cdf_double_slashes (input_name.ds_string);
-	      file_hdr.c_namesize = strlen (file_hdr.c_name) + 1;
+              cpio_set_c_name (&file_hdr,
+                               add_cdf_double_slashes (input_name.ds_string));
 	    }
 	  else
 	    {
 	      /* We don't mark CDF's in tar files.  We assume the "hidden"
 		 directory will always go into the archive before any of
 		 its files.  */
-	      file_hdr.c_name = input_name.ds_string;
-	      file_hdr.c_namesize = strlen (input_name.ds_string) + 1;
+              cpio_set_c_name (&file_hdr, input_name.ds_string);
 	    }
 #endif
 
@@ -866,8 +864,7 @@ process_copy_out ()
   file_hdr.c_chksum = 0;
 
   file_hdr.c_filesize = 0;
-  file_hdr.c_namesize = 11;
-  file_hdr.c_name = CPIO_TRAILER_NAME;
+  cpio_set_c_name (&file_hdr, CPIO_TRAILER_NAME);
   if (archive_format != arf_tar && archive_format != arf_ustar)
     write_out_header (&file_hdr, out_file_des);
   else
diff --git a/src/cpiohdr.h b/src/cpiohdr.h
index b29e6fb..f4c63be 100644
--- a/src/cpiohdr.h
+++ b/src/cpiohdr.h
@@ -129,5 +129,6 @@ struct cpio_file_stat /* Internal representation of a CPIO header */
   char *c_tar_linkname;
 };
 
+void cpio_set_c_name(struct cpio_file_stat *file_hdr, char *name);
 
 #endif /* cpiohdr.h */
diff --git a/src/tar.c b/src/tar.c
index a2ce171..e41f89d 100644
--- a/src/tar.c
+++ b/src/tar.c
@@ -282,7 +282,7 @@ read_in_tar_header (struct cpio_file_stat *file_hdr, int in_des)
       if (null_block ((long *) &tar_rec, TARRECORDSIZE))
 #endif
 	{
-	  file_hdr->c_name = CPIO_TRAILER_NAME;
+	  cpio_set_c_name (file_hdr, CPIO_TRAILER_NAME);
 	  return;
 	}
 #if 0
@@ -316,9 +316,11 @@ read_in_tar_header (struct cpio_file_stat *file_hdr, int in_des)
 	}
 
       if (archive_format != arf_ustar)
-	file_hdr->c_name = stash_tar_filename (NULL, tar_hdr->name);
+        cpio_set_c_name (file_hdr, stash_tar_filename (NULL, tar_hdr->name));
       else
-	file_hdr->c_name = stash_tar_filename (tar_hdr->prefix, tar_hdr->name);
+        cpio_set_c_name (file_hdr, stash_tar_filename (tar_hdr->prefix,
+                                                      tar_hdr->name));
+
       file_hdr->c_nlink = 1;
       file_hdr->c_mode = FROM_OCTAL (tar_hdr->mode);
       file_hdr->c_mode = file_hdr->c_mode & 07777;
@@ -398,7 +400,7 @@ read_in_tar_header (struct cpio_file_stat *file_hdr, int in_des)
 	case AREGTYPE:
 	  /* Old tar format; if the last char in filename is '/' then it is
 	     a directory, otherwise it's a regular file.  */
-	  if (file_hdr->c_name[strlen (file_hdr->c_name) - 1] == '/')
+	  if (file_hdr->c_name[file_hdr->c_namesize - 1] == '/')
 	    file_hdr->c_mode |= CP_IFDIR;
 	  else
 	    file_hdr->c_mode |= CP_IFREG;
diff --git a/src/util.c b/src/util.c
index 6ff6032..4f3c073 100644
--- a/src/util.c
+++ b/src/util.c
@@ -1410,8 +1410,34 @@ set_file_times (int fd,
     utime_error (name);
 }
 
+
+void
+cpio_set_c_name (struct cpio_file_stat *file_hdr, char *name)
+{
+  static size_t buflen = 0;
+  size_t len = strlen (name) + 1;
+
+  if (buflen == 0)
+    {
+      buflen = len;
+      if (buflen < 32)
+        buflen = 32;
+      file_hdr->c_name = xmalloc (buflen);
+    }
+  else if (buflen < len)
+    {
+      buflen = len;
+      file_hdr->c_name = xrealloc (file_hdr->c_name, buflen);
+    }
+
+  file_hdr->c_namesize = len;
+  memmove (file_hdr->c_name, name, len);
+}
+
 /* Do we have to ignore absolute paths, and if so, does the filename
-   have an absolute path?  */
+   have an absolute path?  Before calling this function make sure that the
+   allocated NAME buffer has capacity at least 2 bytes. */
+
 void
 cpio_safer_name_suffix (char *name, bool link_target, bool absolute_names,
 			bool strip_leading_dots)
@@ -1426,6 +1452,10 @@ cpio_safer_name_suffix (char *name, bool link_target, bool absolute_names,
 	  ++p;
       }
   if (p != name)
+    /* The 'p' string is shortened version of 'name' with one exception;  when
+       the 'name' points to an empty string (buffer where name[0] == '\0') the
+       'p' then points to static string ".".  So caller needs to ensure there
+       are at least two bytes available in 'name' buffer so memmove succeeds. */
     memmove (name, p, (size_t)(strlen (p) + 1));
 }
 
diff --git a/tests/inout.at b/tests/inout.at
index 60c3716..730cbd2 100644
--- a/tests/inout.at
+++ b/tests/inout.at
@@ -35,7 +35,22 @@ while read NAME LENGTH
 do
 	genfile --length $LENGTH > $NAME
 	echo $NAME
-done < filelist |
- cpio --quiet -o > archive])
+done < filelist > filelist_raw
+
+for format in bin odc newc crc tar ustar hpbin hpodc
+do
+    cpio --format=$format --quiet -o < filelist_raw > archive.$format
+    rm -rf output
+    mkdir output && cd output
+    cpio -i --quiet < ../archive.$format
+
+    while read file
+    do
+        test -f $file || echo "$file not found"
+    done < ../filelist_raw
+
+    cd ..
+done
+])
 
 AT_CLEANUP
-- 
1.9.1