Re: Crash in 'post' after recent commit - patch in next mail...

[Valdis Klētnieks]

On Mon, 10 May 2021 04:17:44 -0700, David Levine said:
> Valdis wrote:
>
> > which gives free() indigestion. I suspect Fedora is using a hardened 
> > malloc()
> > that does more sanity checking and that's why it didn't get caught right 
> > away.
>
> Or the tests don't cover a path starting with '~'.
>
> > So obviously a more subtle solution is needed.
>
> Yeah, I'll work on that.

I originally put "patch in next mail" in the subject because the fix to not use
a static char[] seemed obvious.  Then I actually ran 'make check' and that blew
up in a way that was pretty obviously a case of somebody doing something subtly
dodgy indeed

Good luck.  Hopefully chasing down why the "obvious" fix breaks isn't *too*
bad.  

Re: Crash in 'post' after recent commit - patch in next mail...

[David Levine]

Valdis wrote:

> which gives free() indigestion. I suspect Fedora is using a hardened malloc()
> that does more sanity checking and that's why it didn't get caught right away.

Or the tests don't cover a path starting with '~'.

> So obviously a more subtle solution is needed.

Yeah, I'll work on that.

Thanks for the detailed report.

David

Crash in 'post' after recent commit - patch in next mail...

[Valdis Klētnieks]

Noticed I hadn't updated since Feb, did a git pull, things went poorly.

The gory details:

git bisect says:
[~/src/nmh] git bisect bad
c723593d2af190d9c86062d2a265fceec25fb777 is the first bad commit
commit c723593d2af190d9c86062d2a265fceec25fb777
Author: David Levine 
Date:   Sun Mar 21 09:31:21 2021 -0400

Added alias expansion to From: address for use by sendfrom.
 
but I think this is merely exposing a bug in this commit:

commit 85aebdf30129dc779d3164a3ba784215283dac26
Author: David Levine 
Date:   Sun Mar 21 09:25:34 2021 -0400

Plugged memory leak resulting from etcpath() call.

Trying to send mail says:

What now? send -snoop
free(): invalid pointer
send: message not delivered to anyone

What now?

And gdb says: 

Core was generated by `post -library /home/valdis/Mail -nomime -msgid -server 
smtp.gmail.com -port 587'.
Program terminated with signal SIGABRT, Aborted.
#0  0x7f023723f292 in raise () from /lib64/libc.so.6
(gdb) where
#0  0x7f023723f292 in raise () from /lib64/libc.so.6
#1  0x7f02372288a4 in abort () from /lib64/libc.so.6
#2  0x7f0237281cd7 in __libc_message () from /lib64/libc.so.6
#3  0x7f023728995c in malloc_printerr () from /lib64/libc.so.6
#4  0x7f023728ad2c in _int_free () from /lib64/libc.so.6
#5  0x7f023728ea58 in free () from /lib64/libc.so.6
#6  0x00408e8d in alias (file=0x439ca0  
"/home/valdis/.mh_alias", 
file@entry=0x7ffd2d74175c "~/.mh_alias") at uip/aliasbr.c:184
#7  0x0040571a in main (argc=, argv=) at 
uip/post.c:388

(gdb) up
#6  0x00408e8d in alias (file=0x439ca0  
"/home/valdis/.mh_alias", 
file@entry=0x7ffd2d74175c "~/.mh_alias") at uip/aliasbr.c:184
184 if (allocated_file) free (file);
(gdb) info local
i = 
bp = 
cp = 
pp = 
lc = 
ap = 0x7ffd2d7416ed "/home/valdis/Mail"
ak = 0x0
allocated_file = true
fp = 0x2419d60

The code:

165 alias (char *file)
166 {
167 int i;
168 char *bp, *cp, *pp;
169 char lc, *ap;
170 struct aka *ak = NULL;
171 bool allocated_file = false;
172 FILE *fp;
173 
174 if (*file != '/'
175 && !has_prefix(file, "./") && !has_prefix(file, "../")) {
176 file = etcpath (file);
177 allocated_file = true;
178 }
179 if ((fp = fopen (file, "r")) == NULL) {
180 akerrst = file;
181 return AK_NOFILE;
182 }
183 
184 if (allocated_file) free (file);

The problem is that if etcpath() is fed ~/some.path, what it returns is a
pointer to:

 33 static char epath[PATH_MAX];

which gives free() indigestion. I suspect Fedora is using a hardened malloc()
that does more sanity checking and that's why it didn't get caught right away.

Just making epath an malloc'ed array produces interesting breakage:

PASS: test/whatnow/test-ls
./test/whom/test-whom: "whom -alias nonexistent" expected:
'whom: aliasing error in nonexistent - unable to read 'nonexistent''
but instead got:
'whom: aliasing error in /usr/lo - unable to read '/usr/lo''
first named test failure: -alias with nonexistent aliasfile
FAIL: test/whom/test-whom

So obviously a more subtle solution is needed.