sync: fix open() fallback bug

Detected by Coverity Analysis:

Error: RESOURCE_LEAK (CWE-772):
coreutils-8.30/src/sync.c:112: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
coreutils-8.30/src/sync.c:112: var_assign: Assigning: "fd" = handle returned from "open(file, 2049)".
coreutils-8.30/src/sync.c:115: leaked_handle: Handle variable "fd" going out of scope leaks the handle.
113|         if (fd < 0)
114|           error (0, rd_errno, _("error opening %s"), quoteaf (file));
115|->       return false;
116|       }
117|

Bug: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33287
This commit is contained in:
Kamil Dudka 2018-11-07 15:59:54 +01:00
parent ed52fbfec9
commit f26bf7c959
2 changed files with 81 additions and 0 deletions

View File

@ -0,0 +1,77 @@
From 2eabfbee57be82f755c74cbb05755dce1469ea7c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 6 Nov 2018 10:35:16 -0800
Subject: [PATCH 1/2] sync: fix open fallback bug
Problem caught by Coverity Analysis
and reported by Kamil Dudka (Bug#33287).
* src/sync.c (sync_arg): Fix typo in fallback code.
Upstream-commit: 94d364f157f007f2b23c70863ac8eefe9b21229d
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
src/sync.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/sync.c b/src/sync.c
index bd3671a..607fa8f 100644
--- a/src/sync.c
+++ b/src/sync.c
@@ -111,8 +111,10 @@ sync_arg (enum sync_mode mode, char const *file)
if (open_flags != (O_WRONLY | O_NONBLOCK))
fd = open (file, O_WRONLY | O_NONBLOCK);
if (fd < 0)
- error (0, rd_errno, _("error opening %s"), quoteaf (file));
- return false;
+ {
+ error (0, rd_errno, _("error opening %s"), quoteaf (file));
+ return false;
+ }
}
/* We used O_NONBLOCK above to not hang with fifos,
--
2.17.2
From e62ff3068f1f1b1e84d3319f54f1b869bb0bf6cc Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <mail@bernhard-voelker.de>
Date: Wed, 7 Nov 2018 00:26:01 +0100
Subject: [PATCH 2/2] sync: add test for the fix in the previous commit
* tests/misc/sync.sh: Add a test with a write-only file for the fix.
Upstream-commit: 4711c49312d54e84996c13c612f7081c95f821a6
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
---
tests/misc/sync.sh | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tests/misc/sync.sh b/tests/misc/sync.sh
index f60d28c..3bb6e17 100755
--- a/tests/misc/sync.sh
+++ b/tests/misc/sync.sh
@@ -19,7 +19,7 @@
. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
print_ver_ sync
-touch file
+touch file || framework_failure_
# fdatasync+syncfs is nonsensical
returns_ 1 sync --data --file-system || fail=1
@@ -30,6 +30,11 @@ returns_ 1 sync -d || fail=1
# Test syncing of file (fsync) (little side effects)
sync file || fail=1
+# Test syncing of write-only file - which failed since adding argument
+# support to sync in coreutils-8.24.
+chmod 0200 file || framework_failure_
+sync file || fail=1
+
# Ensure multiple args are processed and diagnosed
returns_ 1 sync file nofile || fail=1
--
2.17.2

View File

@ -20,6 +20,9 @@ Patch1: coreutils-8.30-renameatu.patch
# fix heap-based buffer overflow in vasnprintf() (CVE-2018-17942)
Patch2: coreutils-8.30-CVE-2018-17942.patch
# sync: fix open() fallback bug
Patch3: coreutils-8.30-fsync-fallback.patch
# disable the test-lock gnulib test prone to deadlock
Patch100: coreutils-8.26-test-lock.patch
@ -256,6 +259,7 @@ fi
%changelog
* Wed Nov 07 2018 Kamil Dudka <kdudka@redhat.com> - 8.30-6
- sync: fix open() fallback bug
- fix implicit declaration warning in coreutils-getgrouplist.patch
* Thu Oct 11 2018 Kamil Dudka <kdudka@redhat.com> - 8.30-5