Possible bug: uninitialized variable in oidc_cache_mutex_post_config

[ErmakovDmitriy]

We have been running mod OpenIDC in a container with Debian 12 base image.

Container images with version 2.4.16.5 fail to start with segfault, see below:

nobody@container:/$ gdb --args /usr/sbin/apache2 -DFOREGROUND
gdb: warning: Couldn't determine a path for the index cache directory.
GNU gdb (Debian 13.1-3) 13.1
...
Reading symbols from /usr/sbin/apache2...
(No debugging symbols found in /usr/sbin/apache2)
(gdb) run
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /usr/sbin/apache2 -DFOREGROUND
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
AH00112: Warning: DocumentRoot [/stub/] does not exist
AH00558: apache2: Could not reliably determine the server's fully qualified domain name, using 172.20.133.69. Set the 'ServerName' directive globally to suppress this message
warning: Temporarily disabling breakpoints for unloaded shared library "/usr/lib/apache2/modules/mod_auth_openidc.so"
AH00112: Warning: DocumentRoot [/stub/] does not exist
AH00558: apache2: Could not reliably determine the server's fully qualified domain name, using 172.20.133.69. Set the 'ServerName' directive globally to suppress this message
[Thu Dec 05 13:30:52.301281 2024] [lbmethod_heartbeat:notice] [pid 149:tid 149] AH02283: Using slotmem from mod_heartmonitor

Breakpoint 1, oidc_cache_mutex_post_config (s=s@entry=0x7ffff7c3d4a0, m=0x7ffff6a8bec0, type=type@entry=0x7ffff7bebe4e "shm") at src/cache/common.c:83
83	apr_byte_t oidc_cache_mutex_post_config(server_rec *s, oidc_cache_mutex_t *m, const char *type) {
(gdb) cont
Continuing.

Breakpoint 2, oidc_cache_mutex_post_config (s=s@entry=0x7ffff7c3d4a0, m=0x7ffff6a8bec0, type=type@entry=0x7ffff7bebe4e "shm") at src/cache/common.c:90
90		m->mutex_filename =
(gdb) 
Continuing.

Breakpoint 1, oidc_cache_mutex_post_config (s=s@entry=0x7ffff7c3d4a0, m=0x7ffff7c43b98, type=type@entry=0x7ffff7bf8fb3 "refresh") at src/cache/common.c:83
83	apr_byte_t oidc_cache_mutex_post_config(server_rec *s, oidc_cache_mutex_t *m, const char *type) {
(gdb) n

Breakpoint 2, oidc_cache_mutex_post_config (s=s@entry=0x7ffff7c3d4a0, m=0x7ffff7c43b98, type=type@entry=0x7ffff7bf8fb3 "refresh") at src/cache/common.c:90
90		m->mutex_filename =
(gdb) list
85		apr_status_t rv = APR_SUCCESS;
86		const char *dir;
87	
88		/* construct the mutex filename */
89		apr_temp_dir_get(&dir, s->process->pool);
90		m->mutex_filename =
91		    apr_psprintf(s->process->pool, "%s/mod_auth_openidc_%s_mutex.%ld.%pp", dir, type, (long int)getpid(), s);
92	
93		/* set the lock type */
94		apr_lockmech_e mech =
(gdb) 
95	#ifdef OIDC_LOCK
96		    OIDC_LOCK
97	#elif APR_HAS_POSIXSEM_SERIALIZE
98		    APR_LOCK_POSIXSEM
99	#else
100		    APR_LOCK_DEFAULT
101	#endif
102		    ;
103	
104		/* create the mutex lock */
(gdb) list oidc_cache_mutex_post_config
78		char buf[OIDC_CACHE_ERROR_STR_MAX];
79		apr_strerror(statcode, buf, OIDC_CACHE_ERROR_STR_MAX);
80		return apr_pstrdup(p, buf);
81	}
82	
83	apr_byte_t oidc_cache_mutex_post_config(server_rec *s, oidc_cache_mutex_t *m, const char *type) {
84	
85		apr_status_t rv = APR_SUCCESS;
86		const char *dir;
87	
(gdb) 
88		/* construct the mutex filename */
89		apr_temp_dir_get(&dir, s->process->pool);
90		m->mutex_filename =
91		    apr_psprintf(s->process->pool, "%s/mod_auth_openidc_%s_mutex.%ld.%pp", dir, type, (long int)getpid(), s);
92	
93		/* set the lock type */
94		apr_lockmech_e mech =
95	#ifdef OIDC_LOCK
96		    OIDC_LOCK
97	#elif APR_HAS_POSIXSEM_SERIALIZE
(gdb) 
98		    APR_LOCK_POSIXSEM
99	#else
100		    APR_LOCK_DEFAULT
101	#endif
102		    ;
103	
104		/* create the mutex lock */
105		if (m->is_global)
106			rv = apr_global_mutex_create(&m->gmutex, (const char *)m->mutex_filename, mech, s->process->pool);
107		else
(gdb) x/s dir
0x1:	<error: Cannot access memory at address 0x1>
(gdb) n

Program received signal SIGSEGV, Segmentation fault.
__strlen_evex () at ../sysdeps/x86_64/multiarch/strlen-evex.S:79
79	../sysdeps/x86_64/multiarch/strlen-evex.S: No such file or directory.

I have tried to debug and it seems that in our configuration (I could only reproduce it in one of our environments, so it is hard to reproduce), function apr_temp_dir_get (https://github.com/OpenIDC/mod_auth_openidc/blob/master/src/cache/common.c#L89) fails to find a temporary dir which makes the value of the dir unpredictable (https://github.com/apache/apr/blob/d131b4e1be4fc45e9f092cff90d7c87baa251e2b/file_io/unix/tempdir.c#L40C57-L40C65).

I patched the code of this function to have it as below:

apr_byte_t oidc_cache_mutex_post_config(server_rec *s, oidc_cache_mutex_t *m, const char *type) {

	apr_status_t rv = APR_SUCCESS;
	const char *dir;

	/* construct the mutex filename */

        // Print result of apr_temp_dir_get as error text message
	apr_status_t retcode = apr_temp_dir_get(&dir, s->process->pool);

	char errbuf[1024] = {0};
	char *errmsg = 0;
	errmsg = apr_strerror(retcode, errbuf, 1024);
	oidc_serror(s, "Temp dir lookup result: %s", errmsg);

	char *tmpname =
	    apr_psprintf(s->process->pool, "%s/mod_auth_openidc_%s_mutex.%ld.%pp", dir, type, (long int)getpid(), s);

	m->mutex_filename = tmpname;

	/* set the lock type */
	apr_lockmech_e mech =
#ifdef OIDC_LOCK
	    OIDC_LOCK
#elif APR_HAS_POSIXSEM_SERIALIZE
	    APR_LOCK_POSIXSEM
#else
	    APR_LOCK_DEFAULT
#endif
	    ;

	/* create the mutex lock */
	if (m->is_global)
		rv = apr_global_mutex_create(&m->gmutex, (const char *)m->mutex_filename, mech, s->process->pool);
	else
		rv = apr_proc_mutex_create(&m->pmutex, (const char *)m->mutex_filename, mech, s->process->pool);

	if (rv != APR_SUCCESS) {
		oidc_serror(
		    s, "apr_global_mutex_create/apr_proc_mutex_create failed to create mutex (%d) on file %s: %s (%d)",
		    mech, m->mutex_filename, oidc_cache_status2str(s->process->pool, rv), rv);
		return FALSE;
	}

	/* need this on Linux */
#ifdef AP_NEED_SET_MUTEX_PERMS
	if (m->is_global) {
#if MODULE_MAGIC_NUMBER_MAJOR >= 20081201
		rv = ap_unixd_set_global_mutex_perms(m->gmutex);
#else
		rv = unixd_set_global_mutex_perms(m->gmutex);
#endif
		if (rv != APR_SUCCESS) {
			oidc_serror(s, "unixd_set_global_mutex_perms failed; could not set permissions: %s (%d)",
				    oidc_cache_status2str(s->process->pool, rv), rv);
			return FALSE;
		}
	}
#endif

	oidc_slog(s, APLOG_TRACE1, "create: %pp (m=%pp,s=%pp, p=%d)", m, m->gmutex ? m->gmutex : 0, s, m->is_parent);

	return TRUE;
}

and when I run the patched module, I got an error:

[Thu Dec 05 15:13:43.547025 2024] [auth_openidc:error] [pid 83:tid 83] oidc_cache_mutex_post_config: Temp dir lookup result: Internal error (specific information not available), see below:

(gdb) run
Starting program: /usr/sbin/apache2 -DFOREGROUND
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
AH00112: Warning: DocumentRoot [/stub/] does not exist
AH00558: apache2: Could not reliably determine the server's fully qualified domain name, using 172.20.133.23. Set the 'ServerName' directive globally to suppress this message
warning: Temporarily disabling breakpoints for unloaded shared library "/usr/lib/apache2/modules/mod_auth_openidc.so"
AH00112: Warning: DocumentRoot [/stub/] does not exist
AH00558: apache2: Could not reliably determine the server's fully qualified domain name, using 172.20.133.23. Set the 'ServerName' directive globally to suppress this message
[Thu Dec 05 15:13:40.597067 2024] [lbmethod_heartbeat:notice] [pid 83:tid 83] AH02283: Using slotmem from mod_heartmonitor

Breakpoint 1, oidc_cache_mutex_post_config (s=s@entry=0x7ffff7c3d4a0, m=0x7ffff6a8bec0, type=type@entry=0x7ffff7bebe4e "shm") at src/cache/common.c:83
83	apr_byte_t oidc_cache_mutex_post_config(server_rec *s, oidc_cache_mutex_t *m, const char *type) {
(gdb) continue
Continuing.

Breakpoint 2, oidc_cache_mutex_post_config (s=s@entry=0x7ffff7c3d4a0, m=0x7ffff6a8bec0, type=type@entry=0x7ffff7bebe4e "shm") at src/cache/common.c:93
93		errmsg = apr_strerror(retcode, errbuf, 1024);
(gdb) 
Continuing.
[Thu Dec 05 15:13:43.547025 2024] [auth_openidc:error] [pid 83:tid 83] oidc_cache_mutex_post_config: Temp dir lookup result: Internal error (specific information not available)

Program received signal SIGSEGV, Segmentation fault.
__strlen_evex () at ../sysdeps/x86_64/multiarch/strlen-evex.S:79
79	../sysdeps/x86_64/multiarch/strlen-evex.S: No such file or directory.
(gdb) bt
#0  __strlen_evex () at ../sysdeps/x86_64/multiarch/strlen-evex.S:79
#1  0x00007ffff7ed6b2f in apr_vformatter () from /lib/x86_64-linux-gnu/libapr-1.so.0
#2  0x00007ffff7ee3d85 in apr_pvsprintf () from /lib/x86_64-linux-gnu/libapr-1.so.0
#3  0x00007ffff7ee41f4 in apr_psprintf () from /lib/x86_64-linux-gnu/libapr-1.so.0
#4  0x00007ffff7baf31d in oidc_cache_mutex_post_config (s=s@entry=0x7ffff7c3d4a0, m=0x7ffff6a8bec0, type=type@entry=0x7ffff7bebe4e "shm") at src/cache/common.c:97
#5  0x00007ffff7bae6ab in oidc_cache_shm_post_config (s=0x7ffff7c3d4a0) at src/cache/shm.c:113
#6  0x00007ffff7ba5953 in oidc_cfg_post_config (cfg=0x7ffff6b43428, s=s@entry=0x7ffff7c3d4a0) at src/cfg/cfg.c:895
#7  0x00007ffff7b9ea9a in oidc_post_config (pool=<optimized out>, p1=<optimized out>, p2=<optimized out>, s=0x7ffff7c3d4a0) at src/mod_auth_openidc.c:1720
#8  0x00005555555b5b04 in ap_run_post_config ()
#9  0x000055555558e18e in main ()
(gdb) 

which probably corresponds to this return https://github.com/apache/apr/blob/d131b4e1be4fc45e9f092cff90d7c87baa251e2b/file_io/unix/tempdir.c#L124 from apr_temp_dir_get.

I think that although my container runtime environment configuration is not well-configured for the mod_oidc (no tmpdir at all) and I will fix it, it might be a good idea to handle the return code of apr_temp_dir_get and fail the module start in a nice way. Also, while I am not an expert, I can imagine a situation that the value of the dir pointer might at some random case point to some critical memory/information because it was not initialized.

[zandbelt]

agreed, we'll improve the error handling and avoid the segfault; thanks for reporting

[ErmakovDmitriy]

Thank you!

[zandbelt]

see a2cfc06, this will be included in the 2.4.16.6 release that is coming very soon

[zandbelt]

https://github.com/OpenIDC/mod_auth_openidc/releases/tag/v2.4.16.6