Skip to content

Commit 97c8a8c

Browse files
committed
Revert "pr27270 and pr27284, ar segfaults and wrong file mode"
This reverts commit 95b91a0. Given the problems associated with this patch and the others intended to fix the smart_rename CVE, the decision has been taken to$
1 parent 86cb5ea commit 97c8a8c

File tree

4 files changed

+28
-50
lines changed

4 files changed

+28
-50
lines changed

binutils/ChangeLog

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,3 @@
1-
2021-02-03 Alan Modra <[email protected]>
2-
3-
PR 27270
4-
PR 27284
5-
PR 26945
6-
* ar.c: Don't include libbfd.h.
7-
(write_archive): Replace xmalloc+strcpy with xstrdup. Use
8-
bfd_stat rather than fstat on iostream. Move stat and fd tests
9-
outside of _WIN32 ifdef. Delete skip_stat variable.
10-
* arsup.c (temp_name, real_ofd): New static variables.
11-
(ar_open): Use make_tempname and bfd_fdopenw.
12-
(ar_save): Adjust to suit ar_open changes. Move stat output
13-
of _WIN32 ifdef.
14-
* objcopy.c: Don't include libbfd.h.
15-
(copy_file): Use bfd_stat.
16-
171
2021-01-26 Frederic Cambus <[email protected]>
182

193
* objcopy.c (copy_main): Fix a double free happening when both

binutils/ar.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
#include "sysdep.h"
2727
#include "bfd.h"
28+
#include "libbfd.h"
2829
#include "libiberty.h"
2930
#include "progress.h"
3031
#include "getopt.h"
@@ -1254,8 +1255,10 @@ write_archive (bfd *iarch)
12541255
bfd *contents_head = iarch->archive_next;
12551256
int ofd = -1;
12561257
struct stat target_stat;
1258+
bfd_boolean skip_stat = FALSE;
12571259

1258-
old_name = xstrdup (bfd_get_filename (iarch));
1260+
old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) + 1);
1261+
strcpy (old_name, bfd_get_filename (iarch));
12591262
new_name = make_tempname (old_name, &ofd);
12601263

12611264
if (new_name == NULL)
@@ -1300,9 +1303,11 @@ write_archive (bfd *iarch)
13001303

13011304
#if !defined (_WIN32) || defined (__CYGWIN32__)
13021305
ofd = dup (ofd);
1303-
#endif
1304-
if (ofd == -1 || bfd_stat (iarch, &target_stat) != 0)
1306+
if (iarch == NULL || iarch->iostream == NULL)
1307+
skip_stat = TRUE;
1308+
else if (ofd == -1 || fstat (fileno ((FILE *) iarch->iostream), &target_stat) != 0)
13051309
bfd_fatal (old_name);
1310+
#endif
13061311

13071312
if (!bfd_close (obfd))
13081313
bfd_fatal (old_name);
@@ -1313,7 +1318,7 @@ write_archive (bfd *iarch)
13131318
/* We don't care if this fails; we might be creating the archive. */
13141319
bfd_close (iarch);
13151320

1316-
if (smart_rename (new_name, old_name, ofd, &target_stat, 0) != 0)
1321+
if (smart_rename (new_name, old_name, ofd, skip_stat ? NULL : &target_stat, 0) != 0)
13171322
xexit (1);
13181323
free (old_name);
13191324
free (new_name);

binutils/arsup.c

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@ extern int deterministic;
4242

4343
static bfd *obfd;
4444
static char *real_name;
45-
static char *temp_name;
46-
static int real_ofd;
4745
static FILE *outfile;
4846

4947
static void
@@ -151,24 +149,27 @@ maybequit (void)
151149
void
152150
ar_open (char *name, int t)
153151
{
154-
real_name = xstrdup (name);
155-
temp_name = make_tempname (real_name, &real_ofd);
152+
char *tname;
153+
const char *bname = lbasename (name);
154+
real_name = name;
156155

157-
if (temp_name == NULL)
156+
/* Prepend tmp- to the beginning, to avoid file-name clashes after
157+
truncation on filesystems with limited namespaces (DOS). */
158+
if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name), name, bname) == -1)
158159
{
159-
fprintf (stderr, _("%s: Can't open temporary file (%s)\n"),
160+
fprintf (stderr, _("%s: Can't allocate memory for temp name (%s)\n"),
160161
program_name, strerror(errno));
161162
maybequit ();
162163
return;
163164
}
164165

165-
obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
166+
obfd = bfd_openw (tname, NULL);
166167

167168
if (!obfd)
168169
{
169170
fprintf (stderr,
170171
_("%s: Can't open output archive %s\n"),
171-
program_name, temp_name);
172+
program_name, tname);
172173

173174
maybequit ();
174175
}
@@ -343,41 +344,28 @@ ar_save (void)
343344
}
344345
else
345346
{
347+
char *ofilename = xstrdup (bfd_get_filename (obfd));
346348
bfd_boolean skip_stat = FALSE;
347349
struct stat target_stat;
348-
int ofd = real_ofd;
350+
int ofd = -1;
349351

350352
if (deterministic > 0)
351353
obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
352354

353355
#if !defined (_WIN32) || defined (__CYGWIN32__)
354356
/* It's OK to fail; at worst it will result in SMART_RENAME using a slow
355357
copy fallback to write the output. */
356-
ofd = dup (ofd);
358+
ofd = dup (fileno ((FILE *) obfd->iostream));
359+
if (lstat (real_name, &target_stat) != 0)
360+
skip_stat = TRUE;
357361
#endif
358-
bfd_close (obfd);
359362

360-
if (lstat (real_name, &target_stat) != 0)
361-
{
362-
/* The temp file created in ar_open has mode 0600 as per mkstemp.
363-
Create the real empty output file here so smart_rename will
364-
update the mode according to the process umask. */
365-
obfd = bfd_openw (real_name, NULL);
366-
if (obfd == NULL
367-
|| bfd_stat (obfd, &target_stat) != 0)
368-
skip_stat = TRUE;
369-
if (obfd != NULL)
370-
{
371-
bfd_set_format (obfd, bfd_archive);
372-
bfd_close (obfd);
373-
}
374-
}
363+
bfd_close (obfd);
375364

376-
smart_rename (temp_name, real_name, ofd,
365+
smart_rename (ofilename, real_name, ofd,
377366
skip_stat ? NULL : &target_stat, 0);
378367
obfd = 0;
379-
free (temp_name);
380-
free (real_name);
368+
free (ofilename);
381369
}
382370
}
383371

binutils/objcopy.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
#include "sysdep.h"
2222
#include "bfd.h"
23+
#include "libbfd.h"
2324
#include "progress.h"
2425
#include "getopt.h"
2526
#include "libiberty.h"
@@ -3768,7 +3769,7 @@ copy_file (const char *input_filename, const char *output_filename, int ofd,
37683769
/* To allow us to do "strip *" without dying on the first
37693770
non-object file, failures are nonfatal. */
37703771
ibfd = bfd_openr (input_filename, input_target);
3771-
if (ibfd == NULL || bfd_stat (ibfd, in_stat) != 0)
3772+
if (ibfd == NULL || fstat (fileno ((FILE *) ibfd->iostream), in_stat) != 0)
37723773
{
37733774
bfd_nonfatal_message (input_filename, NULL, NULL, NULL);
37743775
status = 1;

0 commit comments

Comments
 (0)