Bug 52832 - numerical configuration entry can be mistakenly interpreted without users' awareness
Summary: numerical configuration entry can be mistakenly interpreted without users' aw...
Status: NEW
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: Core (show other bugs)
Version: 2.4.1
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-05 23:06 UTC by Jiaqi Zhang
Modified: 2014-04-02 11:48 UTC (History)
3 users (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jiaqi Zhang 2012-03-05 23:06:40 UTC
The configuration parsing logic for numerical entries almost all use atoi() or atol() to convert a string to an integer (for example, in set_max_ranges(), set_max_reversals(), etc.) 

However, atoi and atol will only return error (<=0) if the initial portion of the string is not digit. In case the string starts with some digits, they will convert the initial portions without reporting any error.

For example, the string "2o0" will be parsed and returned as 2. In this case, although the users' original intention might be set it to 200, the program executes as it is 2 instead, without giving out any warnings.

It seems it affects almost all versions of httpd, and every place where atoi / atol is used to convert configuration parameters from string to integer.

Bug fix proposal: use strtol(char* nptr, char **endptr, int base) instead, and give out warnings if the endptr does not point to the end of the string. Or use apr_strtoi64() instead.
Comment 1 Stefan 2012-03-08 01:56:13 UTC
hmmm... interesting. I agree with this point.

although it seems a quite stupid mistake to have 20O instead of 200, the correct system response is to check because human beings can make any kinds of mistakes.

actually in a lot of modules, Apache does use strtol to check this problem.
Comment 2 Tianyin Xu 2012-03-20 21:00:16 UTC
(In reply to comment #0)
> The configuration parsing logic for numerical entries almost all use atoi() or
> atol() to convert a string to an integer (for example, in set_max_ranges(),
> set_max_reversals(), etc.) 
> 
> However, atoi and atol will only return error (<=0) if the initial portion of
> the string is not digit. In case the string starts with some digits, they will
> convert the initial portions without reporting any error.
> 
> For example, the string "2o0" will be parsed and returned as 2. In this case,
> although the users' original intention might be set it to 200, the program
> executes as it is 2 instead, without giving out any warnings.
> 
> It seems it affects almost all versions of httpd, and every place where atoi /
> atol is used to convert configuration parameters from string to integer.
> 
> Bug fix proposal: use strtol(char* nptr, char **endptr, int base) instead, and
> give out warnings if the endptr does not point to the end of the string. Or use
> apr_strtoi64() instead.


another problem I can think about is the integer overflow.

Being fed with a overflowed number, atoi() will return a random number.

for example, on my machine, ind b = atoi(10000000000), b will be 1410065408. 

It seems that Apache httpd heavily uses atoi() without carefully check. Just randomly pick one example as follows:

//in server/listen.c
AP_DECLARE_NONSTD(const char *) ap_set_send_buffer_size(cmd_parms *cmd,
                                                        void *dummy,
                                                        const char *arg)
{
    int s = atoi(arg);
    const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);

    if (err != NULL) {
        return err;
    }

    if (s < 512 && s != 0) {
        return "SendBufferSize must be >= 512 bytes, or 0 for system default.";
    }

    send_buffer_size = s;
    return NULL;
}

I think a good software should be able to check and have correct response instead of keeping silent.
Comment 3 Olaf van der Spek 2014-04-02 11:48:02 UTC
(In reply to Tianyin Xu from comment #2)
> Being fed with a overflowed number, atoi() will return a random number.
> 
> for example, on my machine, ind b = atoi(10000000000), b will be 1410065408. 

Actually it's worse: "If the value cannot be represented, the behavior is undefined."

http://pubs.opengroup.org/onlinepubs/009695399/functions/atoi.html

Code using strtol often fails to check for ERANGE too.
IMO the best solution would be to introduce a better strtol wrapper that returns an int/error on invalid input.