When compiling with -O3 gcc finds an interesting error:
src/ar.c: In function ‘do_oper_insert’:
src/ar.c:1077:56: error: ‘%-*ld’ directive output may be truncated writing
between 6 and 10 bytes into a region of size 7 [-Werror=format-truncation=]
snprintf (tmpbuf, sizeof (tmpbuf), ofmt ? "%-*lo" : "%-*ld", bufsize, val);
^
The problem is that the ar header values have to fit in a limited
(not zero terminated) string. We should check the snprintf return
value to see if the values are representable.
Also make ar valgrind and ubsan clean and add a minimal sanity test.
Reported-by: Matthias Klose
Signed-off-by: Mark Wielaard
---
src/ChangeLog | 9
src/ar.c | 66 ++-
tests/ChangeLog | 6 +
tests/Makefile.am | 4 ++--
tests/run-ar.sh | 40 +
5 files changed, 103 insertions(+), 22 deletions(-)
create mode 100755 tests/run-ar.sh
diff --git a/src/ChangeLog b/src/ChangeLog
index daedfca..3c34026 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,12 @@
+2017-09-10 Mark Wielaard
+
+ * ar.c (do_oper_delete): Remove DEBUG conditional check.
+ (no0print): Return bool. Check snprintf return value.
+ (do_oper_insert): Initialize elf. Remove DEBUG conditional check.
+ Check no0print calls succeed. Explicitly elf_end found elfs.
+ (do_oper_extract): Make sure we don't create an empty variable
+ length array.
+
2017-05-04 Ulf Hermann
* stack.c: Print pid_t using %lld.
diff --git a/src/ar.c b/src/ar.c
index ec32cee..818115b 100644
--- a/src/ar.c
+++ b/src/ar.c
@@ -1,5 +1,5 @@
/* Create, modify, and extract from archives.
- Copyright (C) 2005-2012, 2016 Red Hat, Inc.
+ Copyright (C) 2005-2012, 2016, 2017 Red Hat, Inc.
This file is part of elfutils.
Written by Ulrich Drepper , 2005.
@@ -442,7 +442,7 @@ static int
do_oper_extract (int oper, const char *arfname, char **argv, int argc,
long int instance)
{
- bool found[argc];
+ bool found[argc > 0 ? argc : 1];
memset (found, '\0', sizeof (found));
size_t name_max = 0;
@@ -1056,13 +1056,11 @@ do_oper_delete (const char *arfname, char **argv, int
argc,
goto nonew_unlink;
errout:
-#ifdef DEBUG
elf_end (elf);
arlib_fini ();
close (fd);
-#endif
not_found (argc, argv, found);
@@ -1070,12 +1068,18 @@ do_oper_delete (const char *arfname, char **argv, int
argc,
}
-static void
+/* Prints the given value in the given buffer without a trailing zero char.
+ Returns false if the given value doesn't fit in the given buffer. */
+static bool
no0print (bool ofmt, char *buf, int bufsize, long int val)
{
char tmpbuf[bufsize + 1];
- snprintf (tmpbuf, sizeof (tmpbuf), ofmt ? "%-*lo" : "%-*ld", bufsize, val);
+ int ret = snprintf (tmpbuf, sizeof (tmpbuf), ofmt ? "%-*lo" : "%-*ld",
+ bufsize, val);
+ if (ret >= (int) sizeof (tmpbuf))
+return false;
memcpy (buf, tmpbuf, bufsize);
+ return true;
}
@@ -1084,7 +1088,7 @@ do_oper_insert (int oper, const char *arfname, char
**argv, int argc,
const char *member)
{
int status = 0;
- Elf *elf;
+ Elf *elf = NULL;
struct stat st;
int fd = open_archive (arfname, O_RDONLY, 0, &elf, &st, oper != oper_move);
@@ -1303,13 +1307,11 @@ do_oper_insert (int oper, const char *arfname, char
**argv, int argc,
if (status != 0)
{
-#ifdef DEBUG
elf_end (elf);
arlib_fini ();
close (fd);
-#endif
return status;
}
@@ -1463,14 +1465,36 @@ do_oper_insert (int oper, const char *arfname, char
**argv, int argc,
memcpy (arhdr.ar_name, tmpbuf, sizeof (arhdr.ar_name));
}
- no0print (false, arhdr.ar_date, sizeof (arhdr.ar_date),
- all->sec);
- no0print (false, arhdr.ar_uid, sizeof (arhdr.ar_uid), all->uid);
- no0print (false, arhdr.ar_gid, sizeof (arhdr.ar_gid), all->gid);
- no0print (true, arhdr.ar_mode, sizeof (arhdr.ar_mode),
- all->mode);
- no0print (false, arhdr.ar_size, sizeof (arhdr.ar_size),
- all->size);
+ if (! no0print (false, arhdr.ar_date, sizeof (arhdr.ar_date),
+ all->sec))
+ {
+ error (0, errno, gettext ("cannot represent ar_date"));
+ goto nonew_unlink;
+ }
+ if (! no0print (false, arhdr.ar_uid, sizeof (arhdr.ar_uid),
+ all->uid))
+ {
+ error (0, errno, gettext ("cannot represent ar_uid"));
+ goto nonew_unlink;
+ }
+ if (! no0print (false, arhdr.ar_gid, sizeof (arhdr.ar_gid),
+ all->gid))
+ {
+ error (0, err
