Integer config attributes accept invalid values at server startup

[389-ds-bot]

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/47790

• Created at 2014-04-29 23:04:57 by nhosoi (@nhosoi)
• Closed as Fixed
• Assigned to mreynolds (@mreynolds389)
• Associated bugzillas
  • https://bugzilla.redhat.com/show_bug.cgi?id=1092342

---

Description of problem:
nsslapd-ndn-cache-max-size accepts any invalid value.

Version-Release number of selected component (if applicable):
389-ds-base-1.3.1.6-25.el7.x86_64

How reproducible:
Always

Steps to Reproduce:

1. ldapmodify -h localhost -p 389 -D "cn=directory manager" -w ***** <<EOF
   dn: cn=config
   changetype: modify
   replace: nsslapd-ndn-cache-max-size
   nsslapd-ndn-cache-max-size: ~
   EOF

2. systemctl restart dirsrv@dhcp201-149

Actual results:
DS does not give any error and operation is successful.

Expected results:
Error is expected.

[389-ds-bot]

Comment from nhosoi (@nhosoi) at 2014-04-29 23:14:34

The input string "value" is converted to long integer by atol, which does not check the error, but returns 0 when the input is invalid. The set function then adjusts the size to the default size, and it returns SUCCESS. I think this is the right behaviour. Probably, we could log it in the error log, but I don't think we need to return an error there.

int
config_set_ndn_cache_max_size(const char *attrname, char *value, char *errorbuf, int apply )
{
    [...]
    size = atol(value);
    if(size < 0){
        size = 0; /* same as -1 */
    }

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2014-07-15 22:23:17

There was schema mistake that set nsslapd-ndn-cache-max-size as a Directory String, and not an Integer. However, manually editing the dse.ldif file allows invalid values to be set. The following patch addresses this scenario.

[389-ds-bot]

Comment from nhosoi (@nhosoi) at 2014-07-15 23:21:25

Not sure if we need this helper function: config_value_is_number(char *value)...

Is this not enough for checking the validity?

man strtol[l]
ERRORS EINVAL (not in C99) The given base contains an unsupported value.

And I haven't checked it, but there's no param which could take a negative value?

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2014-07-15 23:35:13

Replying to [comment:6 nhosoi]:

Not sure if we need this helper function: config_value_is_number(char *value)...

Is this not enough for checking the validity?

man strtol[l]
ERRORS EINVAL (not in C99) The given base contains an unsupported value.

And I haven't checked it, but there's no param which could take a negative value?

Some parameters take "-1" as a value for unlimited. So strtol() complains about negative values, as does my current fix. So I have some more work to do...

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2014-07-15 23:39:55

Replying to [comment:7 mreynolds389]:

Replying to [comment:6 nhosoi]:

Not sure if we need this helper function: config_value_is_number(char *value)...

Is this not enough for checking the validity?

man strtol[l]
ERRORS EINVAL (not in C99) The given base contains an unsupported value.

And I haven't checked it, but there's no param which could take a negative value?

Some parameters take "-1" as a value for unlimited. So strtol() complains about negative values, as does my current fix. So I have some more work to do...

Actually strtol() does accept negative numbers, reworking patch...

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2014-07-16 00:32:13

revision
0001-Ticket-47790-Integer-config-attributes-accept-invali.patch

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2014-07-16 00:33:21

New patch attached...

[389-ds-bot]

Comment from nhosoi (@nhosoi) at 2014-07-16 00:39:47

Thanks, Mark!!

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2014-07-16 01:57:33

git merge config_test
Updating 8b305c1..d58a568
Fast-forward
ldap/schema/01core389.ldif | 2 +-
ldap/servers/slapd/libglobs.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------

git push origin master
8b305c1..d58a568 master -> master

commit d58a568
Author: Mark Reynolds mreynolds389@redhat.com
Date: Tue Jul 15 14:07:56 2014 -0400

[389-ds-bot]

Comment from nhosoi (@nhosoi) at 2017-02-11 23:12:09

Metadata Update from @nhosoi:

• Issue assigned to mreynolds389
• Issue set to the milestone: 1.3.3 - 8/14 (August)