andy/glusterfs
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

coverity: Fix remaining SECURE_TEMP issues reported

Browse Source 
Two pending SECURE_TEMP issues still exist in the coverity
reports, these are fixed by this patch.

In both instances (where functions actually seem to be
duplicates of each other) the need was for a FILE * and
not an fd. Applied the same pattern in both places as in
other parts of the code where mkstemp was used and later
a FILE * was created from the resulting fd for use.

Coverity report: https://download.gluster.org/pub/gluster/
  glusterfs/static-analysis/master/glusterfs-coverity/
  2018-07-30-4d3c62e7/html/

Issues numbered: 382, 383 (named SECURE_TEMP)

Further added tmpfile to the blacklist, so that future code
changes do not add the same, into symbol-check.sh.

Also corrected shellcheck errors in symbol-check.sh as a
result of updating the same.

Updates: bz#789278

Change-Id: I1d572a16ca5b5df2f597aeaa5f454fad34c8296e
Signed-off-by: ShyamsundarR <srangana@redhat.com>
This commit is contained in:
ShyamsundarR 2018-07-30 14:09:14 -04:00 committed by Amar Tumballi
parent 9e96d64653
commit 60f1aeb08d
4 changed files with 90 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
api/src/glfs-mgmt.c
View File

@ -582,6 +582,8 @@ glfs_mgmt_getspec_cbk (struct rpc_req *req, struct iovec *iov, int count,
struct glfs *fs = NULL;
dict_t *dict = NULL;
char *servers_list = NULL;
int tmp_fd = -1;
char template[] = "/tmp/gfapi.volfile.XXXXXX";
frame = myframe;
ctx = frame->this->ctx;
@ -668,11 +670,28 @@ volfile:
goto out;
}
tmpfp = tmpfile ();
if (!tmpfp) {
ret = -1;
goto out;
}
/* coverity[secure_temp] mkstemp uses 0600 as the mode and is safe */
tmp_fd = mkstemp (template);
if (-1 == tmp_fd) {
ret = -1;
goto out;
}
/* Calling unlink so that when the file is closed or program
* terminates the temporary file is deleted.
*/
ret = sys_unlink (template);
if (ret < 0) {
gf_msg (frame->this->name, GF_LOG_INFO, 0, API_MSG_VOLFILE_INFO,
"Unable to delete file: %s", template);
ret = 0;
}
tmpfp = fdopen (tmp_fd, "w+b");
if (!tmpfp) {
ret = -1;
goto out;
}
fwrite (rsp.spec, size, 1, tmpfp);
fflush (tmpfp);
@ -706,6 +725,7 @@ volfile:
ret = glfs_process_volfp (fs, tmpfp);
/* tmpfp closed */
tmpfp = NULL;
tmp_fd = -1;
if (ret)
goto out;
@ -745,6 +765,8 @@ out:
if (tmpfp)
fclose (tmpfp);
else if (tmp_fd != -1)
sys_close (tmp_fd);
return 0;
}

3
glusterfsd/src/glusterfsd-messages.h
View File

@ -61,7 +61,8 @@ GLFS_MSGID(GLUSTERFSD,
glusterfsd_msg_35,
glusterfsd_msg_36,
glusterfsd_msg_37,
glusterfsd_msg_38
glusterfsd_msg_38,
glusterfsd_msg_39
);
#endif /* !_GLUSTERFSD_MESSAGES_H_ */

32
glusterfsd/src/glusterfsd-mgmt.c
View File

@ -1901,6 +1901,8 @@ mgmt_getspec_cbk (struct rpc_req *req, struct iovec *iov, int count,
char sha256_hash[SHA256_DIGEST_LENGTH] = {0, };
dict_t *dict = NULL;
char *servers_list = NULL;
int tmp_fd = -1;
char template[] = "/tmp/glfs.volfile.XXXXXX";
frame = myframe;
ctx = frame->this->ctx;
@ -1990,7 +1992,32 @@ volfile:
}
}
tmpfp = tmpfile ();
/* coverity[secure_temp] mkstemp uses 0600 as the mode and is
* safe
*/
tmp_fd = mkstemp (template);
if (-1 == tmp_fd) {
gf_msg (frame->this->name, GF_LOG_ERROR, 0,
glusterfsd_msg_39,
"Unable to create temporary file: %s",
template);
ret = -1;
goto out;
}
/* Calling unlink so that when the file is closed or program
* terminates the temporary file is deleted.
*/
ret = sys_unlink (template);
if (ret < 0) {
gf_msg (frame->this->name, GF_LOG_INFO, 0,
glusterfsd_msg_39,
"Unable to delete temporary file: %s",
template);
ret = 0;
}
tmpfp = fdopen (tmp_fd, "w+b");
if (!tmpfp) {
ret = -1;
goto out;
@ -2036,6 +2063,7 @@ volfile:
ret = glusterfs_process_volfp (ctx, tmpfp);
/* tmpfp closed */
tmpfp = NULL;
tmp_fd = -1;
if (ret)
goto out;
@ -2103,6 +2131,8 @@ out:
if (tmpfp)
fclose (tmpfp);
else if (tmp_fd != -1)
sys_close (tmp_fd);
return 0;
}

33
tests/basic/symbol-check.sh
View File

@ -13,6 +13,8 @@ syscalls32=$'creat\nfallocate\nftruncate\n__fxstat\n__fxstatat\n\
lseek\n__lxstat\nopenat\nreaddir\nstatvfs\ntruncate\nstat\n\
preadv\npwritev\npread\npwrite'
glibccalls=$'tmpfile'
exclude_files=$'/libglusterfs/src/.libs/libglusterfs_la-syscall.o\n\
/libglusterfs/src/.libs/libglusterfs_la-gen_uuid.o\n\
/contrib/fuse-util/fusermount.o\n\
@ -33,13 +35,14 @@ function main()
done
local retval=0
local t=$(nm ${1} | grep " U " | sed -e "s/ //g" -e "s/ U //g")
local t
t=$(nm "${1}" | grep " U " | sed -e "s/ //g" -e "s/ U //g")
for symy in ${t}; do
for symx in ${syscalls}; do
if [[ ${symx} = ${symy} ]]; then
if [[ ${symx} = "${symy}" ]]; then
case ${symx} in
"creat64") sym="creat";;
@ -70,12 +73,36 @@ function main()
for symx in ${syscalls32}; do
if [[ ${symx} = ${symy} ]]; then
if [[ ${symx} = "${symy}" ]]; then
echo "${1} was not compiled with -D_FILE_OFFSET_BITS=64" >&2
retval=1
fi
done
symy_glibc=$(echo "${symy}" | sed -e "s/@@GLIBC.*//g")
# Eliminate false positives, check if we have a GLIBC symbol in 'y'
if [[ ${symy} != "${symy_glibc}" ]]; then
for symx in ${glibccalls}; do
if [[ ${symx} = "${symy_glibc}" ]]; then
case ${symx} in
"tmpfile") alt="mkstemp";;
*) alt="none";;
esac
if [[ ${alt} = "none" ]]; then
echo "${1} should not call ${symy_glibc}";
else
echo "${1} should use ${alt} instead of ${symy_glibc}" >&2;
fi
retval=1
fi
done
fi
done
if [ ${retval} = 1 ]; then