NULL pointer deref in pcf_check_dbms_client() with unreadable map file

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

NULL pointer deref in pcf_check_dbms_client() with unreadable map file

Andreas Hasenack
Hi,

postfix-3.3.0

we got a bug report (https://bugs.launchpad.net/ubuntu/+source/postfix/+bug/1753470) where postconf was crashing if main.cf had a map pointing to a file that the user couldn't read.

ubuntu@bionic-postfix:~$ l /etc/postfix/valiases.cf
-rw-r----- 1 root root 169 May  7 14:08 /etc/postfix/valiases.cf

ubuntu@bionic-postfix:~$ cat /etc/postfix/valiases.cf
cat: /etc/postfix/valiases.cf: Permission denied

ubuntu@bionic-postfix:~$ postconf
Segmentation fault (core dumped)

ubuntu@bionic-postfix:~$ 

gdb shows the crash is in vstream_fileno(fp):
Program received signal SIGSEGV, Segmentation fault.
pcf_check_dbms_client (cf_file=0x5555557cf7c6 "/etc/postfix/valiases.cf", dp=<optimized out>) at postconf_dbms.c:184
184             if (fstat(vstream_fileno(fp), &st) == 0 && !S_ISREG(st.st_mode)) {
(gdb) p fp
$1 = (VSTREAM *) 0x0
(gdb) p errno
$2 = 13


Just before there is a check for fp == 0, but looking for errno != EACCES, and EACCES is exactly what I got:
        if ((fp = vstream_fopen(cf_file, O_RDONLY, 0)) == 0
            && errno != EACCES) {
            msg_warn("open \"%s\" configuration \"%s\": %m",
                     dp->db_type, cf_file);
            myfree(dict_spec);
            return;
        }


Reply | Threaded
Open this post in threaded view
|

Patch: NULL pointer deref in pcf_check_dbms_client() with unreadable map file

Viktor Dukhovni
On Tue, May 08, 2018 at 10:39:56AM -0300, Andreas Hasenack wrote:

> Just before there is a check for fp == 0, but looking for errno != EACCES,
> and EACCES is exactly what I got:
>         if ((fp = vstream_fopen(cf_file, O_RDONLY, 0)) == 0
>             && errno != EACCES) {
>             msg_warn("open \"%s\" configuration \"%s\": %m",
>                      dp->db_type, cf_file);
>             myfree(dict_spec);
>             return;
>         }

I think the intent was:

diff --git a/src/postconf/postconf_dbms.c b/src/postconf/postconf_dbms.c
index 707bafa5..eddeab0a 100644
--- a/src/postconf/postconf_dbms.c
+++ b/src/postconf/postconf_dbms.c
@@ -174,10 +174,10 @@ static void pcf_check_dbms_client(const PCF_DBMS_INFO *dp, const char *cf_file)
  */
  dict = dict_ht_open(dict_spec, O_CREAT | O_RDWR, 0);
  dict_register(dict_spec, dict);
- if ((fp = vstream_fopen(cf_file, O_RDONLY, 0)) == 0
-    && errno != EACCES) {
-    msg_warn("open \"%s\" configuration \"%s\": %m",
-     dp->db_type, cf_file);
+ if ((fp = vstream_fopen(cf_file, O_RDONLY, 0)) == 0) {
+    if (errno != EACCES)
+ msg_warn("open \"%s\" configuration \"%s\": %m",
+ dp->db_type, cf_file);
     myfree(dict_spec);
     return;
  }

--
        Viktor.
Reply | Threaded
Open this post in threaded view
|

Re: Patch: NULL pointer deref in pcf_check_dbms_client() with unreadable map file

Wietse Venema
Viktor Dukhovni:
> - if ((fp = vstream_fopen(cf_file, O_RDONLY, 0)) == 0
> -    && errno != EACCES) {
> -    msg_warn("open \"%s\" configuration \"%s\": %m",
> -     dp->db_type, cf_file);
> + if ((fp = vstream_fopen(cf_file, O_RDONLY, 0)) == 0) {
> +    if (errno != EACCES)
> + msg_warn("open \"%s\" configuration \"%s\": %m",
> + dp->db_type, cf_file);

Ack.

        Wietse