Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heap overflow leading to crash #6842

Closed
bentley opened this issue Jun 26, 2018 · 11 comments
Closed

Heap overflow leading to crash #6842

bentley opened this issue Jun 26, 2018 · 11 comments
Labels

Comments

@bentley
Copy link

@bentley bentley commented Jun 26, 2018

OpenBSD’s malloc implementation has the ability to detect heap overflows. With the ‘Canaries’ option in malloc.conf, OpenTTD crashes:

openttd(57524) in free(): chunk canary corrupted 0x18973ea0fa80 0x1a@0x1a
openttd(57524) in malloc(): recursive call
Abort trap (core dumped) 

(gdb) bt
#0  thrkill () at -:3
#1  0x0000067e6c421d9e in _libc_abort () at /usr/src/lib/libc/stdlib/abort.c:51
#2  0x0000067e6c453119 in wrterror (d=0x67e07e784d0, 
    msg=0x67e6c5d1a22 "chunk canary corrupted %p %#tx@%#zx%s")
    at /usr/src/lib/libc/stdlib/malloc.c:291
#3  0x0000067e6c4561ea in validate_canary (d=<optimized out>, 
    ptr=<optimized out>, sz=1, allocated=<optimized out>)
    at /usr/src/lib/libc/stdlib/malloc.c:1020
#4  find_chunknum (d=0x0, info=<optimized out>, ptr=0x0, check=<optimized out>)
    at /usr/src/lib/libc/stdlib/malloc.c:1045
#5  0x0000067e6c453753 in ofree (argpool=<optimized out>, p=<optimized out>, 
    clear=0, check=0, argsz=0) at /usr/src/lib/libc/stdlib/malloc.c:1359
#6  0x0000067e6c45321c in free (ptr=0x67e3e90ba20)
    at /usr/src/lib/libc/stdlib/malloc.c:1419
#7  0x0000067b7afd6739 in free (ptr=0x67e3e90ba20)
    at /usr/ports/pobj/openttd-1.8.0/openttd-1.8.0/src/stdafx.h:508
#8  DetermineBasePaths (exe=0x7f7ffffdcc78 "/usr/local/bin/openttd")
    at /usr/ports/pobj/openttd-1.8.0/openttd-1.8.0/src/fileio.cpp:1073
#9  0x0000067b7afd6a82 in DeterminePaths (exe=0x0)
    at /usr/ports/pobj/openttd-1.8.0/openttd-1.8.0/src/fileio.cpp:1173
#10 0x0000067b7b0e3a9f in openttd_main (argc=<optimized out>, 
    argv=0x7f7ffffdcad8)
    at /usr/ports/pobj/openttd-1.8.0/openttd-1.8.0/src/openttd.cpp:700
#11 0x0000067b7af016a6 in _start ()
@LordAro

This comment has been minimized.

Copy link
Member

@LordAro LordAro commented Jun 28, 2018

I don't know the details of how the canaries option works, I wonder if it just doesn't like the free (const void*) override.

The xdg stuff itself looks fine to me.. https://github.com/OpenTTD/OpenTTD/blob/release/1.8/src/fileio.cpp#L1070-L1073

@frosch123

This comment has been minimized.

Copy link
Member

@frosch123 frosch123 commented Jun 29, 2018

The xdgDataHome function had exactly this type of bug in 2013/2014:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=730487
Apparently it was fixed in libxdg-basedir-1.2.0. Does (your) OpenBSD have this fix?

@TrueBrain

This comment has been minimized.

Copy link
Member

@TrueBrain TrueBrain commented Aug 5, 2018

@bentley friendly poke; can you check if OpenBSD has the libxdg fix, that could potentially solve the crash?

@bentley

This comment has been minimized.

Copy link
Author

@bentley bentley commented Aug 7, 2018

Thanks for the reminder. We have libxdg-basedir-1.2.0 already.

@stale

This comment has been minimized.

Copy link

@stale stale bot commented Mar 25, 2019

This issue has been automatically marked as stale because it has not had any activity in the last two months.
If you believe the issue is still relevant, please test on the latest nightly and report back.
It will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale bot added the stale label Mar 25, 2019
@bentley

This comment has been minimized.

Copy link
Author

@bentley bentley commented Mar 26, 2019

This crash is still present in 1.9.0-RC1.

@stale stale bot removed the stale label Mar 26, 2019
@LordAro

This comment has been minimized.

Copy link
Member

@LordAro LordAro commented Dec 31, 2019

We cannot reproduce this, nor find any sign that any actual memory leakage occurs. I'm afraid the ball is now in your court to debug further - be it through valgrind or gdb

@bentley

This comment has been minimized.

Copy link
Author

@bentley bentley commented Jan 2, 2020

@bentley

This comment has been minimized.

Copy link
Author

@bentley bentley commented Jan 3, 2020

Okay, this is indeed the libxdg-basedir bug, which was not fixed in 1.2.0 (the latest release, nearly eight years ago). Sorry for taking your time on this.

@bentley bentley closed this Jan 3, 2020
@LordAro

This comment has been minimized.

Copy link
Member

@LordAro LordAro commented Jan 3, 2020

Thanks very much for confirming that. Presumably a bug report in libxdg-basedir is required? :)

@LordAro

This comment has been minimized.

Copy link
Member

@LordAro LordAro commented Jan 3, 2020

Minor update - seems the fix was only included as a debian patch, not actually upstream as we originally thought - https://sources.debian.org/patches/libxdg-basedir/1.2.0-2/

Hope that helps

(Realistically we probably want to move away from this library given how unmaintained it is)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.