#47835 Coverity: 12687..12692
Closed: wontfix Opened by nhosoi.

12692: Use of untrusted string value
12687..12691: Unbounded source buffer


git patch file (master) -- 12692 - Use of untrusted string value
0001-Ticket-47835-Coverity-12687.12692.patch

Reviewed by Rich (Thank you!!)

Pushed to master:
b6b7199..43c6ff2 master -> master
commit 43c6ff2e7801ff6bbc03961b3161dd60aebf707a
commit 0a546bcb3d4625d6db1dcbb342922b4ddb3bee37
commit 48f2ea020bca3ee750885a6e5f423fae5e3606ca
commit 162604a620ba75f2a5eed3095930a2aaa823a645
commit f25c7f1f988783d620171f7b648f946dc6704c81
commit 8dc3806d75b6e3d4722047e230db68ac20ab3e69

I would like to ask what is wrong with calling strdup. It should be safe.
I saw many replacement of strdup in patches. In my opinion, it is a false positive.

For example:
{{{
diff --git a/ldap/servers/slapd/tools/dbscan.c b/ldap/servers/slapd/tools/dbscan.c
index 023fade..bbfcd0e 100644
--- a/ldap/servers/slapd/tools/dbscan.c
+++ b/ldap/servers/slapd/tools/dbscan.c
@@ -1077,16 +1077,17 @@ is_changelog(char *filename)

static void usage(char argv0)
{
- char
copy = strdup(argv0);
+ long arg_max = sysconf(_SC_ARG_MAX);
+ char copy = strndup(argv0, arg_max);
}}}
Variable ''argv0'' has type ''char
''. So it is nul terminated string.

The manual page says:
{{{
The strdup() function returns a pointer to a new string which is a
duplicate of the string s. Memory for the new string is obtained with
malloc(3), and can be freed with free(3).
}}}

I cannot see a reason why there should be any problem. Could you explain me it? I would like to learn something new.

Yes, it could be false positive. I'm just worried the case when the given string is corrupted/tainted (e.g., no NULL terminated).

We had this code for years and have not heard of any security issue. But we thought it was a good practice to set the maximum length to the string to duplicate not to overflow anything... At least, you see no harm on that, don't you?

I'm not sure if a string passed from an environment variable, or from a command line argument, can really be non-NULL terminated. We should do some research on that, and also possibly find out what Coverity users usually do to fix such issues.

I cannot say it was a complete research :p, but it was based upon the search result on the coverity error and fix... I don't mind undoing the changes and just mark "false positive" on coverity if that's better.

One line fix:
13ec1d6..6e175e3 master -> master
commit 6e175e3308f04de528254871f6b554fe992dd3df
Description: arg_max has to be always set. But introduced by
commit 0a546bcb3d4625d6db1dcbb342922b4ddb3bee37

It turned out these defects are false positive.
Reverting...
commit 43c6ff2e7801ff6bbc03961b3161dd60aebf707a
commit 0a546bcb3d4625d6db1dcbb342922b4ddb3bee37
commit 48f2ea020bca3ee750885a6e5f423fae5e3606ca
commit 162604a620ba75f2a5eed3095930a2aaa823a645
commit f25c7f1f988783d620171f7b648f946dc6704c81
commit 8dc3806d75b6e3d4722047e230db68ac20ab3e69
commit 6e175e3308f04de528254871f6b554fe992dd3df

Reverted:
daf4b42..3a66ec7 master -> master
{{{
commit 3a66ec7ca61ee273eb016fe234a3da8202e58e83
This reverts commit 43c6ff2e7801ff6bbc03961b3161dd60aebf707a.
commit dc4527b6f6ebe7a2c42eb57942af535c31c5cd59
This reverts commit 0a546bcb3d4625d6db1dcbb342922b4ddb3bee37.
commit f5014300dcdf46d9e432e1a8cee640595b862d6f
This reverts commit 48f2ea020bca3ee750885a6e5f423fae5e3606ca.
commit 50654969bcfeb0aa379c10ceda3cfd72993a527c
This reverts commit 162604a620ba75f2a5eed3095930a2aaa823a645.
commit cef82090cc3ece1a72c2ec1b62205b1f68b1074f
This reverts commit f25c7f1f988783d620171f7b648f946dc6704c81.
commit 8247976f25c22799a31be08074cc150e07f5dcce
This reverts commit 8dc3806d75b6e3d4722047e230db68ac20ab3e69.
commit 4b66c032996c7a313202f315683aa75e83d5f361
This reverts commit 6e175e3308f04de528254871f6b554fe992dd3df.
}}}

Metadata Update from @nhosoi:
- Issue set to the milestone: 1.3.3 backlog

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1166

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Invalid)

Metadata