| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
| |
If there is a problem allocating pResHash, we jump to the error
label. The error label then dereferences pResHash to do a deep
free, but it doesn't check if pResHash is NULL first. We need to
check if pResHash is NULL before dereferencing it.
|
|
|
|
|
|
|
| |
At the end of the for loop, be will be NULL if we never find a
valid be->be_usn_counter. This will cause us to dereference a
NULL pointer at the next if statement after the for loop. We
need to check if be is NULL before dereferencing it.
|
|
|
|
|
|
|
|
| |
If the attr parameter that is passed to my_ber_scanf_attr() is
NULL, we jump to the loser label where we clean up memory we may
have allocated. We dereference attr without first checking if it
is NULL in this clean-up code. We need to check if attr is NULL
before dereferencing it.
|
|
|
|
|
|
|
|
|
|
|
|
| |
Coverity believes that search_result_pb can be NULL since we check
if it is NULL before freeing the internal search results. If this
was true, there would be a NULL dereference issue when we call
slapi_pblock_get(). We are guaranteed that search_result_pb is
non-NULL after slapi_pblock_new() is called since the server would
exit if it was unable to allocate memory.
We should remove the NULL check before freeing the internal search
results.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If we fail to fetch the postop entry for a modrdn operation in the
Managed Entry Plug-in, we end up passing a NULL pointer to
slapi_entry_attr_get_charptr(). This function dereferences the
entry without checking if it is NULL first. The mep_modrdn_post_op()
function should just return if we are unable to fetch the postop
entry.
I believe that this issue could trigger a crash when chain-on-update
is configured and a modrdn operation is chained. There is no postop
entry in this case.
|
|
|
|
|
|
|
|
| |
It is not necessary to check if config_entry->types is NULL since
it is guaranteed to be non-NULL by dna_parse_config_entry() when
it creates config_entry. Coverity thinks that a NULL derefence is
possible since we are checking if config_entry->types is NULL. We
should remove this NULL check.
|
|
|
|
|
|
| |
When parsing a URL without a host or port present, we can
dereference a NULL pointer. We need to check if hostport is NULL
before dereferencing it.
|
|
|
|
|
|
| |
The entry pointer that is passed to slapi_entry_attr_find() is
dereferenced by that function without a check for NULL. We should
check if ep->ep_entry is NULL before calling slapi_entry_attr_find().
|
|
|
|
|
|
| |
We need to check if ruv is NULL before dereferencing it. The
assertion will not help us here in an optimized build, so an
explicit NULL check will keep us from crashing.
|
|
|
|
|
| |
The tmpDn pointer is deferenced before checking if it is NULL. We
need to check if it is NULL first.
|
|
|
|
|
|
|
|
| |
The libaccess library has some dead functions it it. One of these
functions was flagged as having a NULL pointer dereference issue
by Coverity. The problem function is unused, so it should be removed.
There are also a number of other unused functions in the same source
file that should be removed.
|
|
|
|
|
|
|
|
| |
If we jump to the error label due to an error allocating memory
for pResHash->treelist, we try to do a free of
pResHash->treelist->vlist without checking if pResHash->treelist
is NULL. We need to perform this NULL check before dereferencing
pResHash->treelist.
|
|
|
|
|
|
|
| |
There is a chance that we can deference a NULL pointer in the
mmldif code. If "(numb > tot_b)" is true, it is not guaranteed
that "a" is non-NULL. We need to check if "a" is NULL before
dereferencing it in the "(cmp < 0)" case.
|
|
|
|
|
|
|
|
|
|
|
|
| |
There is a possibility of deferencing prevocp when it is NULL
the second time through the loop if the first pass was not a
standard objectclass definition and tmpocp != curlisthead.
I don't think that this issue is possible unless some other
thread was able to modify tmpocp->oc_next between where curlisthead
is set (schema.c:2654) and where nextocp is set (schema.c:2658) the
first time through the loop. That said, I see no harm in checking
if prevocp is NULL before attempting to dereference it.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
https://bugzilla.redhat.com/show_bug.cgi?id=631862
Resolves: bug 631862
Bug Description: crash - delete entries not in cache + referint
Reviewed by: rmeggins and nhosoi
Branch: master
Fix Description: When deleting an entry, the referential integrity (referint)
plugin does an internal search to find references to this entry (e.g. in
group entries) and removes them. The search code wants to ensure that the
entrydn attribute is present in the entry when using entryrdn (subtree
rename). The search code sets a flag to tell the id2entry code to add the
entrydn attribute if it is not present. However, it was doing this to an
entry in the cache, which may be in use by another thread. The solution is
to add the entrydn attribute before adding the entry to the cache. In the
id2entry code, this is after the entry has been read from the id2entry db
successfully, but before the entry is added to the cache. In the LDAP ADD
code, this is done when the other computed operational attributes are added
to the new entry.
In addition to the above fix by rmeggins@redhat.com, following changes are
made:
1) entrydn attribute is always added to the entry in memory before putting
it in the entry cache, and the attribute is removed before writing the
entry to the database.
2) eliminating id2entry_ext, which was introduced to pass flags, but it is
no longer needed since only a flag ID2ENTRY_ADD_ENTRYDN was removed.
Platforms tested: RHEL5 x86_64
Flag Day: no
Doc impact: no
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The function slapi_mapping_tree_select_and_check() is only called for
modrdn operations, to make sure we are not attempting to rename a suffix
or move an entry from one backend to another. This defeats datainterop
plugins that may want to perform some other operation in these cases. If
the target suffix/backend is not found, the default backend is used. If
the default backend is being used, don't check for all errors, just allow
the operation to pass through to the preop plugins.
Need to make sure this doesn't cause problems if
1) null suffix is not used - entry really is bogus or doesn't exist
2) null suffix is being used but entry belongs to another null suffix or
is really bogus
Reviewed by: nhosoi (Thanks!)
Tested on: Fedora 14 x86_64
|
|
|
|
|
|
| |
https://bugzilla.redhat.com/show_bug.cgi?id=633168
Description: previous commit was missing the upgrade script 81changelog.pl
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
https://bugzilla.redhat.com/show_bug.cgi?id=633168
Description:
* cl5_api.c, cl5_api.h
- fetches dbEnv from backend using slapi_back_get_info.
- unused macros and DB helper functions and APIs are removed.
* cl5_config.c
- local changelog DB related config parameters are removed.
* Added SLAPI_PLUGIN_BE_PRE_CLOSE_FN and SLAPI_PLUGIN_BE_POST_OPEN_FN to
close changelog DB before dbEnv is closed and to open changelog DB after
dbEnv is opened, respectively.
* Added slapi APIs slapi_back_get_info and slapi_back_set_info to get/set
the backend info.
* back-ldbm
- db2bak[.pl] and bak2db[.pl] backs up and restores the database files
including changelog db.
- changelog dir is backed up in <backupdir>/.repl_changelog_backup.
- underlying implementation ldbm_back_get_info for slapi_back_get_info
is added.
* Added an upgrade script 81changelog.pl
See also:
http://directory.fedoraproject.org/wiki/Move_changelog
|
|
|
|
|
|
|
| |
Object ro is freed in objset_next_obj and next object is returned
if any. After ro is released, it was used to get agreement data.
This patch moves the location of objset_next_obj after the agreement
data is retrieved.
|
|
|
|
|
|
| |
In attr_index_config(), if argc or argv are NULL, we jump to the
done label. We then try to free attrs, but it was never initialized.
We need to initialize attrs to NULL.
|
|
|
|
|
| |
In search_easter_egg(), we need to initialize the bervals before
we pass them to slapi_ldif_parse_line().
|
|
|
|
|
|
|
|
| |
If we encounter an error early in
ldbm_instance_index_config_modify_callback(), we jump to the out
label where we try to free origMatchingRules, but it may not be
initialized. The same is true for origIndexTypes. We need to
initialize these pointers to NULL.
|
|
|
|
|
|
|
|
| |
If we encounter an error early in
ldbm_instance_index_config_modify_callback(), we jump to the out
label where we free each element of the arglist array. This can
happen without initializing the array. We need to initialize arglist
before there is any chance to jump to the out label.
|
|
|
|
|
|
|
|
| |
In entryrdn_get_parent(), there is a DBT structure that we can use
without initializing. If we goto the bail label, we try to free
data.data, but data was never initialized. We should clear the
memory used by data in the beginning of the function before we have
an opportunity to goto bail.
|
|
|
|
|
|
| |
We use some uninitialized bervals when the backend code calls
slapi_ldif_parse_line(). We should be initializing the bervals to
be empty.
|
|
|
|
|
|
| |
When the server is built against MozLDAP, we use some uninitialized
bervals when the backend code calls slapi_ldif_parse_line(). We
should be initializing the bervals to be empty.
|
|
|
|
|
|
| |
There are a few more unused ACL functions to remove. One of these
unused functions is causing coverity to report an error about
memory corruption.
|
|
|
|
|
|
|
|
|
|
|
| |
The switch statements in agt_mopen_stats() are missing breaks to
prevent falling through to the next case when the stats file is
opened in read-only mode. This looks like it causes the stats file
to get opened a second time in read/write mode when ldap-agent
attempts to open it in read-only mode. This may leak file
descriptors in ldap-agent.
We need to add the proper break statements.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The final frees of priv->memory and priv will never be reached since
the function returns prior to these calls. It looks as if an
"error:" label was removed at some point, as the WIN32 code in this
function has goto statements using that label, but the label is not
defined.
The fix is to add the "error:" label in ifdef blocks for WIN32 that
calls the free of priv. The free of priv->memory is not necessary
since WIN32 doesn't use it and non-WIN32 builds don't use the error
label at all.
|
|
|
|
|
|
|
|
|
|
|
| |
In the call to slapi_log_error(), we are guaranteed that srdn is
NULL if we are checking it for NULL due to the way the conditions
are nested. The only time we check if srdn is NULL is if be is
non-NULL, and the if condition guarantees that either be or srdn
are NULL.
We can just use the string "srdn" in our log message if be is
non-NULL.
|
|
|
|
|
|
|
|
|
|
|
| |
In the moddn code that renames child entries, the for loop used to
rename the children can never be executed. Part of the condition
is that retval is 0, but retval will always be -1 the first time we
hit this loop. This only happens with subtree rename off, but it
should still be fixed.
The fix is to set retval to 0 at the prior to checking the condition
the first time.
|
|
|
|
|
|
|
|
| |
The skipit variable is set to zero shortly before we check if it
is 0 in an if condition. This if block can be removed since it
will never be hit. The entry that was being freed in the if block
is already removed earlier in the function if skipit was non-0
prior to resetting skipit to 0.
|
|
|
|
|
|
|
|
|
|
|
| |
In the call to slapi_log_error(), we are guaranteed that srdn is
NULL if we are checking it for NULL due to the way the conditions
are nested. The only time we check if srdn is NULL is if inst is
non-NULL, and the if condition guarantees that either inst or
srdn are NULL.
We can just use the string "srdn" in our log message if inst is
non-NULL.
|
|
|
|
|
|
|
|
|
| |
If the index types (argv[1]) are not specified, attr_index_config()
bails. We can remove some dead code where we check if "argc == 1"
later in the function since that case can never happen.
Additionally, we need to check if argc is 0, or if argv is NULL
before attempting to parse the list of attributes to be indexed.
|
|
|
|
|
|
|
|
|
|
| |
There is no chance for next_node to be anything other than NULL in
the final return statement due to the return in the "if (next_node)"
block immediately before the final return.
We can remove the return inside of the "if (next_node)" block since
the final return statement already deals with returning the proper
value if next_node is non-NULL.
|
|
|
|
|
|
|
|
| |
There is no chance for local_newentry to be anything other than NULL
when we check it in the call to slapi_log_error() since the check only
happens after we're verified that newparent and local_origsdn are not NULL.
Since we are guaranteed that local_newentry is NULL, we can just eliminate
this check and use the string "local entry" in the message that we print.
|
|
|
|
|
|
|
|
|
|
|
| |
There is no chance for op_string to be NULL if "rc == LDAP_SUCCESS"
since op_string is set for all operation types in the switch statement,
which is the only place that rc can be set to something other that the
value of -1 that it is initialized to.
We can just skip the NULL checking for op_string in the calls to
slapi_log_error(). I also fixed the indentation to help me read
the code easier.
|
|
|
|
|
|
|
|
|
| |
We need to remove the last "if (dnParts)" condition since it will
never be true.
The last frees of newDN, sval, and newvalue are also unnecessary
since they are only set in the non subtree rename case, where they
are already freed as well.
|
|
|
|
|
|
|
|
|
|
|
|
| |
The return statement at the end of agt_mopen_stats() is unreachable
according to coverity. This return was removed before to fix the
coverity defect, but it was added back to fix a compiler warning.
We can satisfy both the compiler and coverity by adding a rc
variable to hold the return code. We can then return rc at the end
of the function. This also allows us to clean up all of the return
calls in this function by having all of them set rc and jump to a
label at the function end.
|
|
|
|
|
|
|
|
|
|
|
| |
The directory variable points to a dynamically allocated string
returned by rel2abspath(). We are changing directory to point to
a string constant if we are unable to parse the directory. This
not only leaks memory, but it can cause us to attempt to free the
string constant.
We should free the string before we overwrite it, and we should
dynamically allocate a new string instead of using a string constant.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Coverity flagged a memory corruption issue in an old unused
ACL function. It is best to just remove these unused functions.
The functions removed are:
ACL_ParseFile
ACL_WriteFile
ACL_WriteString
ACL_Decompose
acl_to_str_*
acl_decompose_*
|
|
|
|
|
|
|
|
|
|
|
| |
We should check the return type of idl_append_extend(), though it does
not seem possible that the return type will be anything other than 0.
The only time idl_append_extend() returns anything other than 0 is
when it is unable to allocate memory. Since the underlying allocation
function is slapi_ch_calloc(), the server will just exit if it runs
out of memory, which means we will never return up through
idl_append_extend(). The right thing to do from a code standpoint is
to still check for the return value though.
|
|
|
|
|
|
|
|
|
|
|
| |
We need to check the return value of cache_replace() in
id2entry_add_ext(). The only possible error that can be returned
is when the entry we are trying to replace is not found in the
cache. This should not occur since we are told that the entry
already exists by CACHE_ADD() just prior to this call. If we run
into this situation, we will just log an error without adding the
entry to the cache. This shouldn't be a big deal since the entry
will get added to the cache next time it is accessed.
|
|
|
|
|
|
| |
We were not checking the return value of stat() before attempting
to access the structure that stat fille in in the protect_db code.
This patch checks the return value first.
|
|
|
|
|
|
|
| |
We were not checking the return value of ber_scanf in the DNA
plug-in when parsing the range transfer response. This checks
the return value and sets the return code to LDAP_PROTOCOL_ERROR
if we were unable to parse the range transfer response.
|
|
|
|
|
|
|
|
|
| |
We were not checking the return value of ldap_parse_result in the
windows_check_user_password() function. The old code was a bit
unclear about setting rc when we encountered errors from
ldap_result(). It also was calling ldap_parse_result() even if
ldap_result() encountered an error. I fixed this code to be a
bit more straightforward.
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, the ldbm_back_ldbm2ldif() function could bail due to an
error before fd is set. We then attempt to close the file that fd
refers to. We should initialize fd to STDOUT_FILENUM, as we skip
calling close() if fd is set to STDOUT_FILENUM.
Additionally, I noticed that we could call close() when fd is
negative or if it is STDERR or STDIN. I fixed this so close() is
not called in those cases.
|
|
|
|
|
|
|
|
|
|
|
| |
The refint plug-in code currently looks as if it could use the
search_result variable when it is uninitialized. I don't believe
that this is possible since it would require the filter variable
to be NULL, which should not occur since slapi_ch_smprintf() would
make the process exit if it failed to allocate memory. Even so,
the correct thing to do from a code cleanliness standpoint is to
move all code that assumes we performed a search into the "if (filter)"
block.
|
|
|
|
|
| |
openldap requires that the protocol version be explicitly set to 3
mozldap defaults to 3, but it doesn't hurt to set it again
|
|
|
|
|
| |
(cov#15521) The oldndn variable was unused in the dncache_replace() function.
This patch removes the unused variable.
|