[389-commits] include/base ldap/servers ldap/systools lib/base lib/libaccess
Richard Allen Megginson
rmeggins at fedoraproject.org
Fri Jan 27 23:52:32 UTC 2012
include/base/file.h | 3 --
ldap/servers/plugins/linkedattrs/fixup_task.c | 7 +++-
ldap/servers/plugins/syntaxes/string.c | 4 +-
ldap/servers/slapd/back-ldbm/index.c | 4 +-
ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c | 38 ++++----------------------
ldap/servers/slapd/libglobs.c | 1
ldap/servers/slapd/ssl.c | 24 ++++++++++------
ldap/servers/slapd/tools/ldclt/data.c | 24 ++++++++--------
ldap/servers/slapd/tools/ldclt/ldapfct.c | 10 ++----
ldap/servers/slapd/tools/ldclt/ldclt.c | 15 ++++++++--
ldap/servers/slapd/tools/ldclt/scalab01.c | 8 ++---
ldap/servers/slapd/tools/ldclt/threadMain.c | 3 +-
ldap/servers/slapd/util.c | 11 ++++++-
ldap/systools/idsktune.c | 6 ++--
lib/base/file.cpp | 24 ----------------
lib/libaccess/acl.tab.cpp | 4 +-
lib/libaccess/acltext.y | 4 +-
17 files changed, 83 insertions(+), 107 deletions(-)
New commits:
commit 24731b3d13029d651c6fd24056b1268d3dcee8b6
Author: Rich Megginson <rmeggins at redhat.com>
Date: Thu Jan 26 12:13:54 2012 -0700
Ticket #161 - Review and address latest Coverity issues
https://fedorahosted.org/389/ticket/161
Resolves: Ticket #161
Bug Description: Review and address latest Coverity issues
Reviewed by: nhosoi (Thanks!)
Branch: master
Fix Description:
12486 12485 12484 Resource leak In
linked_attrs_add_backlinks_callback(): Leak of memory or pointers to
system resources
the return -1 left targetsdn, targets, and pb dangling
allocate targetsdn after shutdown check - instead of return -1, goto done and free resources at done:
12481 Resource leak In config_set_default_naming_context(): Leak of
memory or pointers to system resources
free suffix before return
12477 Uninitialized pointer read In index_addordel_entry(): Reads an
uninitialized pointer or its target
use LDBM_PARENTID_STR instead of type
12476 Dereference after null check In string_assertion2keys_ava():
Pointer is checked against null but then dereferenced anyway
check for NULL val
12475 Logically dead code In _entryrdn_insert_key(): Code can never be
reached because of a logical contradiction
get rid of dead code
12448 Time of check time of use In INTdir_create_all(): A check occurs
on a file's attributes before the file is used in a privileged
operation, but things may have changed
get rid of unused code
12447-12444 Time of check time of use
use open() to open the file, then use the functions that take an fd to
further test or access the file - this prevents someone changing the file
between functions that only use the filename
12434-12425 Copy into fixed size buffer
use strncpy or snprintf and make sure the string is null terminated
Platforms tested: RHEL6 x86_64, Fedora 16
Flag Day: no
Doc impact: no
diff --git a/include/base/file.h b/include/base/file.h
index 496c33e..fa53eb5 100644
--- a/include/base/file.h
+++ b/include/base/file.h
@@ -86,8 +86,6 @@ NSAPI_PUBLIC SYS_DIRENT *INTdir_read(SYS_DIR ds);
NSAPI_PUBLIC void INTdir_close(SYS_DIR ds);
#endif /* XP_WIN32 */
-NSAPI_PUBLIC int INTdir_create_all(char *dir);
-
/* --- OBSOLETE ----------------------------------------------------------
* The following macros/functions are obsolete and are only maintained for
* compatibility. Do not use them. 11-19-96
@@ -131,7 +129,6 @@ NSPR_END_EXTERN_C
#define dir_read INTdir_read
#define dir_close INTdir_close
#endif /* XP_WIN32 */
-#define dir_create_all INTdir_create_all
/* Obsolete */
#ifdef XP_WIN32
diff --git a/ldap/servers/plugins/linkedattrs/fixup_task.c b/ldap/servers/plugins/linkedattrs/fixup_task.c
index b92c6b7..c1af2f2 100644
--- a/ldap/servers/plugins/linkedattrs/fixup_task.c
+++ b/ldap/servers/plugins/linkedattrs/fixup_task.c
@@ -369,12 +369,14 @@ linked_attrs_add_backlinks_callback(Slapi_Entry *e, void *callback_data)
for (i = 0; targets && targets[i]; ++i) {
char *targetdn = (char *)targets[i];
int perform_update = 0;
- Slapi_DN *targetsdn = slapi_sdn_new_normdn_byref(targetdn);
+ Slapi_DN *targetsdn = NULL;
if (g_get_shutdown()) {
- return -1;
+ rc = -1;
+ goto done;
}
+ targetsdn = slapi_sdn_new_normdn_byref(targetdn);
if (config->scope) {
/* Check if the target is within the scope. */
perform_update = slapi_dn_issuffix(targetdn, config->scope);
@@ -408,6 +410,7 @@ linked_attrs_add_backlinks_callback(Slapi_Entry *e, void *callback_data)
slapi_sdn_free(&targetsdn);
}
+done:
slapi_ch_array_free(targets);
slapi_pblock_destroy(pb);
diff --git a/ldap/servers/plugins/syntaxes/string.c b/ldap/servers/plugins/syntaxes/string.c
index f482723..3840c2e 100644
--- a/ldap/servers/plugins/syntaxes/string.c
+++ b/ldap/servers/plugins/syntaxes/string.c
@@ -678,8 +678,8 @@ string_assertion2keys_ava(
break;
case LDAP_FILTER_EQUALITY:
(*ivals) = (Slapi_Value **) slapi_ch_malloc( 2 * sizeof(Slapi_Value *) );
- (*ivals)[0] = slapi_value_dup( val );
- if (!(flags & SLAPI_ATTR_FLAG_NORMALIZED)) {
+ (*ivals)[0] = val ? slapi_value_dup( val ) : NULL;
+ if (val && !(flags & SLAPI_ATTR_FLAG_NORMALIZED)) {
/* 3rd arg: 1 - trim leading blanks */
value_normalize_ext( (*ivals)[0]->bv.bv_val, syntax, 1, &alt );
if (alt) {
diff --git a/ldap/servers/slapd/back-ldbm/index.c b/ldap/servers/slapd/back-ldbm/index.c
index d00fd14..65ad867 100644
--- a/ldap/servers/slapd/back-ldbm/index.c
+++ b/ldap/servers/slapd/back-ldbm/index.c
@@ -379,7 +379,7 @@ index_addordel_entry(
back_txn *txn
)
{
- char *type;
+ char *type = NULL;
Slapi_Value **svals;
int rc, result;
Slapi_Attr *attr;
@@ -430,7 +430,7 @@ index_addordel_entry(
slapi_entry_attr_find(e->ep_entry, LDBM_PARENTID_STR, &attr);
if (attr) {
svals = attr_get_present_values(attr);
- result = index_addordel_values_sv(be, type, svals, NULL,
+ result = index_addordel_values_sv(be, LDBM_PARENTID_STR, svals, NULL,
e->ep_id, flags, txn);
if ( result != 0 ) {
ldbm_nasty(errmsg, 1020, result);
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
index 953129e..40b2f1e 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_entryrdn.c
@@ -2278,6 +2278,7 @@ _entryrdn_insert_key(backend *be,
goto bail;
/* done */
} else {
+ ID currid = 0;
rc = _entryrdn_get_tombstone_elem(cursor, tmpsrdn, &key,
childnrdn, &tmpelem);
if (rc || (NULL == tmpelem)) {
@@ -2296,41 +2297,14 @@ _entryrdn_insert_key(backend *be,
goto bail;
}
/* Node is a tombstone. */
- ID currid = 0;
slapi_ch_free((void **)&elem);
elem = tmpelem;
currid = id_stored_to_internal(elem->rdn_elem_id);
- if (0 == rdnidx) { /* Child is a Leaf RDN to be added */
- if (currid == id) {
- /* already in the file */
- /* do nothing and return. */
- rc = 0;
- slapi_log_error(SLAPI_LOG_BACKLDBM, ENTRYRDN_TAG,
- "_entryrdn_insert_key: ID %d is already "
- "in the index. NOOP.\n", currid);
- } else { /* different id, error return */
- char *dn = NULL;
- int tmprc = slapi_rdn_get_dn(srdn, &dn);
- slapi_log_error(SLAPI_LOG_FATAL,
- ENTRYRDN_TAG,
- "_entryrdn_insert_key: Same DN (%s: %s) "
- "is already in the %s file with different ID "
- "%d. Expected ID is %d.\n",
- tmprc?"rdn":"dn", tmprc?childnrdn:dn,
- LDBM_ENTRYRDN_STR, currid, id);
- slapi_ch_free_string(&dn);
- /* returning special error code for the upgrade */
- rc = LDBM_ERROR_FOUND_DUPDN;
- }
- slapi_ch_free((void **)&tmpelem);
- goto bail;
- } else { /* if (0 != rdnidx) */
- nrdn = childnrdn;
- workid = currid;
- slapi_ch_free((void **)&parentelem);
- parentelem = elem;
- elem = NULL;
- }
+ nrdn = childnrdn;
+ workid = currid;
+ slapi_ch_free((void **)&parentelem);
+ parentelem = elem;
+ elem = NULL;
}
} else {
char *dn = NULL;
diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c
index fa179ae..3e8a615 100644
--- a/ldap/servers/slapd/libglobs.c
+++ b/ldap/servers/slapd/libglobs.c
@@ -5899,6 +5899,7 @@ config_set_default_naming_context(const char *attrname,
}
if (!apply) {
+ slapi_ch_free_string(&suffix);
return LDAP_SUCCESS;
}
diff --git a/ldap/servers/slapd/ssl.c b/ldap/servers/slapd/ssl.c
index e90e00a..f656df9 100644
--- a/ldap/servers/slapd/ssl.c
+++ b/ldap/servers/slapd/ssl.c
@@ -1497,7 +1497,6 @@ char* slapd_get_tmp_dir()
unsigned ilen;
char pch;
#endif
- struct stat ffinfo;
tmp[0] = '\0';
@@ -1535,25 +1534,32 @@ char* slapd_get_tmp_dir()
}
#endif
- if(stat(tmpdir, &ffinfo) == -1)
#if defined( XP_WIN32 )
- if(CreateDirectory(tmpdir, NULL) == 0)
- {
- slapi_log_error(
+ if(CreateDirectory(tmpdir, NULL) == 0)
+ {
+ slapi_log_error(
SLAPI_LOG_FATAL,
"slapd_get_tmp_dir",
"CreateDirectory(%s, NULL) Error: %s\n",
tmpdir, strerror(errno));
- }
+ }
#else
- if(mkdir(tmpdir, 00770) == -1)
- {
+ if(mkdir(tmpdir, 00770) == -1)
+ {
+ if (errno == EEXIST) {
+ slapi_log_error(
+ SLAPI_LOG_TRACE,
+ "slapd_get_tmp_dir",
+ "mkdir(%s, 00770) - already exists\n",
+ tmpdir);
+ } else {
slapi_log_error(
SLAPI_LOG_FATAL,
"slapd_get_tmp_dir",
"mkdir(%s, 00770) Error: %s\n",
- tmpdir, strerror(errno));
+ tmpdir, strerror(errno));
}
+ }
#endif
return ( tmpdir );
}
diff --git a/ldap/servers/slapd/tools/ldclt/data.c b/ldap/servers/slapd/tools/ldclt/data.c
index 33de017..e0eac55 100644
--- a/ldap/servers/slapd/tools/ldclt/data.c
+++ b/ldap/servers/slapd/tools/ldclt/data.c
@@ -234,18 +234,8 @@ int loadImages (
/*
* Read the image size
*/
- strcpy (name, dirpath);
- strcat (name, "/");
- strcat (name, fileName);
- if (stat (name, &stat_buf) < 0)
- {
- perror (name);
- fprintf (stderr, "Cannot stat(%s)\n", name);
- fflush (stderr);
- rc = -1;
- goto exit;
- }
- mctx.images[mctx.imagesNb-1].length = stat_buf.st_size;
+ snprintf (name, sizeof(name), "%s/%s", dirpath, fileName);
+ name[sizeof(name)-1] = '\0';
/*
* Open the image
@@ -260,6 +250,16 @@ int loadImages (
goto exit;
}
+ if (fstat (fd, &stat_buf) < 0)
+ {
+ perror (name);
+ fprintf (stderr, "Cannot stat(%s)\n", name);
+ fflush (stderr);
+ rc = -1;
+ goto exit;
+ }
+ mctx.images[mctx.imagesNb-1].length = stat_buf.st_size;
+
#ifdef _WIN32
/*
* Allocate buffer and read the data :-(
diff --git a/ldap/servers/slapd/tools/ldclt/ldapfct.c b/ldap/servers/slapd/tools/ldclt/ldapfct.c
index fd291bb..76fc9c2 100644
--- a/ldap/servers/slapd/tools/ldclt/ldapfct.c
+++ b/ldap/servers/slapd/tools/ldclt/ldapfct.c
@@ -2624,9 +2624,8 @@ doRename (
*/
if (buildRandomRdnOrFilter (tttctx) < 0)
return (-1);
- strcpy (oldDn, tttctx->bufFilter);
- strcat (oldDn, ",");
- strcat (oldDn, tttctx->bufBaseDN);
+ snprintf (oldDn, sizeof(oldDn), "%s,%s", tttctx->bufFilter, tttctx->bufBaseDN);
+ oldDn[sizeof(oldDn)-1] = '\0';
/*
* Now, build a random new name for this entry
@@ -3585,9 +3584,8 @@ doDeleteEntry (
*/
if (buildRandomRdnOrFilter (tttctx) < 0)
return (-1);
- strcpy (delDn, tttctx->bufFilter);
- strcat (delDn, ",");
- strcat (delDn, tttctx->bufBaseDN);
+ snprintf(delDn, sizeof(delDn), "%s,%s", tttctx->bufFilter, tttctx->bufBaseDN);
+ delDn[sizeof(delDn)-1] = '\0';
ret = ldap_delete_ext_s (tttctx->ldapCtx, delDn, NULL, NULL);
if (ret != LDAP_SUCCESS)
diff --git a/ldap/servers/slapd/tools/ldclt/ldclt.c b/ldap/servers/slapd/tools/ldclt/ldclt.c
index cabdea8..a00058b 100644
--- a/ldap/servers/slapd/tools/ldclt/ldclt.c
+++ b/ldap/servers/slapd/tools/ldclt/ldclt.c
@@ -1270,6 +1270,7 @@ basicInit (void)
int ret; /* Return value */
int oflags;/* open() flags */ /*JLS 05-04-01*/
struct stat file_st ; /* file status checker for attreplacefile option */
+ int fd = -1;
FILE *attrF; /* file pointer for attreplacefile option */
char buffer[BUFFERSIZE]; /* buffer used to read attreplacefile content */
@@ -1539,9 +1540,18 @@ basicInit (void)
(3) save the content into mctx.attrplFileContent
*/
+ fd = open(mctx.attrplFile, O_RDONLY);
+ if (fd == -1)
+ {
+ printf("ERROR reading attr file [%s]\n",mctx.attrplFile);
+ return (-1);
+ }else{
+ printf("file opened for reading\n");
+ }
/* determine file size here */
- if (stat(mctx.attrplFile, &file_st) < 0){
+ if (fstat(fd, &file_st) < 0){
printf ("attr replace file [%s] does not exist, exit\n", mctx.attrplFile);
+ close(fd);
return (-1);
}else{
mctx.attrplFileSize = file_st.st_size;
@@ -1549,9 +1559,10 @@ basicInit (void)
}
/* open file to read */
- if ((attrF = fopen(mctx.attrplFile, "r")) == NULL )
+ if ((attrF = fdopen(fd, "r")) == NULL )
{
printf("ERROR reading attr file [%s]\n",mctx.attrplFile);
+ close(fd);
return (-1);
}else{
printf("file opened for reading\n");
diff --git a/ldap/servers/slapd/tools/ldclt/scalab01.c b/ldap/servers/slapd/tools/ldclt/scalab01.c
index 049e5f6..e64734a 100644
--- a/ldap/servers/slapd/tools/ldclt/scalab01.c
+++ b/ldap/servers/slapd/tools/ldclt/scalab01.c
@@ -429,7 +429,8 @@ scalab01_addLogin (
return -1;
}
- strcpy (new->dn, dn);
+ strncpy (new->dn, dn, sizeof(new->dn));
+ new->dn[sizeof(new->dn)-1] = '\0';
new->cost = new->counter = duration;
new->next = NULL;
@@ -537,9 +538,8 @@ scalab01_connectSuperuser (void)
unsigned int mod2 = mctx.mod2;
if (!(mode & CLTAUTH)) {
- strcpy (bindDN, SCALAB01_SUPER_USER_RDN);
- strcat (bindDN, ",");
- strcat (bindDN, mctx.baseDN);
+ snprintf (bindDN, sizeof(bindDN), "%s,%s", SCALAB01_SUPER_USER_RDN, mctx.baseDN);
+ bindDN[sizeof(bindDN)-1] = '\0';
}
/* clear bits not applicable to this mode */
mod2 &= ~M2_RNDBINDFILE;
diff --git a/ldap/servers/slapd/tools/ldclt/threadMain.c b/ldap/servers/slapd/tools/ldclt/threadMain.c
index f6a2428..1d2ed59 100644
--- a/ldap/servers/slapd/tools/ldclt/threadMain.c
+++ b/ldap/servers/slapd/tools/ldclt/threadMain.c
@@ -627,7 +627,8 @@ msgIdAdd (
tttctx->lastMsgId->next = NULL;
tttctx->lastMsgId->msgid = msgid;
strcpy (tttctx->lastMsgId->str, str);
- strcpy (tttctx->lastMsgId->dn, dn);
+ strncpy (tttctx->lastMsgId->dn, dn, sizeof(tttctx->lastMsgId->dn));
+ tttctx->lastMsgId->dn[sizeof(tttctx->lastMsgId->dn)-1] = '\0';
tttctx->lastMsgId->attribs = attribs;
return (0);
diff --git a/ldap/servers/slapd/util.c b/ldap/servers/slapd/util.c
index fd5ebcb..9cd3656 100644
--- a/ldap/servers/slapd/util.c
+++ b/ldap/servers/slapd/util.c
@@ -50,6 +50,7 @@
#include <pwd.h>
#include <stdint.h>
#endif
+#include <fcntl.h>
#include <libgen.h>
#include <pk11func.h>
#include "slap.h"
@@ -876,21 +877,27 @@ strarray2str( char **a, char *buf, size_t buflen, int include_quotes )
int
slapd_chown_if_not_owner(const char *filename, uid_t uid, gid_t gid)
{
+ int fd = -1;
struct stat statbuf;
int result = 1;
if (!filename)
return result;
+ fd = open(filename, O_RDONLY);
+ if (fd == -1) {
+ return result;
+ }
memset(&statbuf, '\0', sizeof(statbuf));
- if (!(result = stat(filename, &statbuf)))
+ if (!(result = fstat(fd, &statbuf)))
{
if (((uid != -1) && (uid != statbuf.st_uid)) ||
((gid != -1) && (gid != statbuf.st_gid)))
{
- result = chown(filename, uid, gid);
+ result = fchown(fd, uid, gid);
}
}
+ close(fd);
return result;
}
#endif
diff --git a/ldap/systools/idsktune.c b/ldap/systools/idsktune.c
index 2382259..a16c27c 100644
--- a/ldap/systools/idsktune.c
+++ b/ldap/systools/idsktune.c
@@ -2731,7 +2731,8 @@ static int check_fs_options(char *reqdir,char mntbuf[MAXPATHLEN])
if (mntdir_matches(reqdir,reqlen,mep->IDDS_MNTENT_DIRNAME,mntbuf) == 0) {
found = 0;
#if defined(IDDS_MNTENT_OPTIONS)
- strcpy(optbuf,mep->IDDS_MNTENT_OPTIONS);
+ strncpy(optbuf,mep->IDDS_MNTENT_OPTIONS,sizeof(optbuf));
+ optbuf[sizeof(optbuf)-1] = '\0';
#else
strcpy(optbuf,"");
#endif
@@ -3318,7 +3319,8 @@ int main(int argc,char *argv[])
flag_carrier = 1;
break;
case 'i':
- strcpy(install_dir,optarg);
+ strncpy(install_dir,optarg,sizeof(install_dir));
+ install_dir[sizeof(install_dir)-1] = '\0';
break;
default:
usage(argv[0]);
diff --git a/lib/base/file.cpp b/lib/base/file.cpp
index c7a1a9e..6dc39e6 100644
--- a/lib/base/file.cpp
+++ b/lib/base/file.cpp
@@ -503,30 +503,6 @@ NSAPI_PUBLIC int file_notfound(void)
#endif
}
-NSAPI_PUBLIC int dir_create_all(char *dir)
-{
- struct stat fi;
- char *t;
-
-#ifdef XP_WIN32
- t = dir + 3;
-#else /* XP_UNIX */
- t = dir + 1;
-#endif
- while(1) {
- t = strchr(t, FILE_PATHSEP);
- if(t) *t = '\0';
- if(stat(dir, &fi) == -1) {
- if(dir_create(dir) == -1)
- return -1;
- }
- if(t) *t++ = FILE_PATHSEP;
- else break;
- }
- return 0;
-}
-
-
#ifdef XP_UNIX
#if !defined(SNI) && !defined(LINUX)
extern char *sys_errlist[];
diff --git a/lib/libaccess/acl.tab.cpp b/lib/libaccess/acl.tab.cpp
index ad828ac..27c8c1b 100644
--- a/lib/libaccess/acl.tab.cpp
+++ b/lib/libaccess/acl.tab.cpp
@@ -1213,8 +1213,8 @@ case 13:
if (!use_generic_rights) {
acl_string_lower(aclpvt[-0].string);
- strcpy(acl_tmp_arg, "http_");
- strcat(acl_tmp_arg, aclpvt[-0].string);
+ snprintf(acl_tmp_arg, sizeof(acl_tmp_arg), "http_%s", aclpvt[-0].string);
+ acl_tmp_arg[sizeof(acl_tmp_arg)-1] = '\0';
PERM_FREE(aclpvt[-0].string);
acl_new_arg = PERM_STRDUP(acl_tmp_arg);
acl_add_arg(curr_args_list, acl_new_arg);
diff --git a/lib/libaccess/acltext.y b/lib/libaccess/acltext.y
index 368e7ad..6ec9567 100644
--- a/lib/libaccess/acltext.y
+++ b/lib/libaccess/acltext.y
@@ -350,8 +350,8 @@ arg_v2: ACL_VARIABLE_TOK
if (!use_generic_rights) {
acl_string_lower($<string>1);
- strcpy(acl_tmp_arg, "http_");
- strcat(acl_tmp_arg, $<string>1);
+ snprintf(acl_tmp_arg, sizeof(acl_tmp_arg), "http_%s", $<string>1);
+ acl_tmp_arg[sizeof(acl_tmp_arg)-1] = '\0';
PERM_FREE($<string>1);
acl_new_arg = PERM_STRDUP(acl_tmp_arg);
acl_add_arg(curr_args_list, acl_new_arg);
More information about the 389-commits
mailing list