Bug 61193 - segfault if AuthLDAPCharsetConfig file has no utf8 entry
Summary: segfault if AuthLDAPCharsetConfig file has no utf8 entry
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_authnz_ldap (show other bugs)
Version: 2.4.25
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-16 13:10 UTC by Jakob Hirsch
Modified: 2018-03-25 08:03 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jakob Hirsch 2017-06-16 13:10:44 UTC
I need to use AuthLDAPCharsetConfig to convert usernames (and passwords) with German umlauts into UTF-8 for AD authentication. Since my distribution (Fedora 25) didn't provide a charset.conv, I created my own one, only containing "de ISO-8859-1" (which should suffice for our internal use), but unfortunately, after enabling this httpd crashes at startup. A quick glance with strace show that this is probably related:

# strace httpd -X
...
open("/etc/httpd/LDAPCharsetConfig", O_RDONLY|O_CLOEXEC) = 8
fstat(8, {st_mode=S_IFREG|0644, st_size=14, ...}) = 0
read(8, "de ISO-8859-1\n", 4096)        = 14
read(8, "", 4096)                       = 0
close(8)                                = 0
--- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_ACCERR, si_addr=0x7f44a8018977} ---
+++ killed by SIGSEGV (core dumped) +++
Segmentation fault (core dumped)

So I went and downloaded the httpd 2.4.25 tar to try the provided charset.conv (from docs/conf), which worked just fine, but left me a little doubtful about the reliability of this config option. After some playing I found out that can trigger the segfault again just by commenting out the line "UTF-8       utf8        UTF-8" (which looked a litte odd to me, as UTF-8 is not a language). Contrary, I can prevent the segfault with my custom charset config by adding a line saying "UTF-8 utf".

btw, after some resarch, I found a similar report on the apache-users list, from 02/2009, so this is probably long standing: http://mail-archives.apache.org/mod_mbox/httpd-users/200902.mbox/%3C49957BF2.5040309%40ofd-sth.niedersachsen.de%3E
Comment 1 Jakob Hirsch 2017-06-16 14:07:11 UTC
sorry, the line should sayd "UTF-8 utf8", not "... utf", obviously.

The reason for the issue is probably in mod_authnz_ldap.c:

authnz_ldap_post_config(), line 1902: 
to_charset = derive_codepage_from_lang (p, "utf-8");

derive_codepage_from_lang() first tries to find the language ("utf-8" in this case). if this fails, it shortens language to two chars by setting "language[2] = '\0';". But language is set to the static string "utf-8", stored in a read-only data segment or with the program code (also read-only), so it cannot be changed. 

Unrelated but notable: Since derive_codepage_from_lang() cannot even know wether language points to memory longer than 2 bytes, this is bad style (at least). You don't want to crash (or worse) just because some client has single or zero char element in his Accept-Language header (didn't check if this is not catched otherwise).

I _guess_ the intention was the provide a fallback for something like "en-US" to simply "en", if the former is not configured, so the logic itself is not that bad, but sloppy implemented. First, shortening "UTF-8" or "Unicode" (examples from conversion.conv) to two chars does not make sense, "UT" or "UN" could be some real language extension. The check in line 129 should at least be extended to something like this:

  if (!charset && strlen(language) > 2 && language[2] == '-') ...

This will not help against somebody calling derive_codepage_from_lang() with a static "xy-ZZ", of course, so to be really safe, we would also need rw memory (something like strndupa).
Comment 2 Luca Toscano 2018-03-25 08:03:19 UTC
Hi Jacob,

thanks a lot for this report and sorry for the time it took to fix it. It was reported independently by another user and its fix ended up in 2.4.32 (unreleased in favor of 2.4.33). 

Fix in 2.4.x: http://svn.apache.org/r1824456

Announcement: http://seclists.org/oss-sec/2018/q1/271 (mentioned your work in there :)

Luca