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.

Sign up for GitHub

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

Jump to bottom

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
bug Something isn't working needs triage This issue needs further investigation before it becomes actionable

Comments

@bentley
Copy link

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
Copy link
Member

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
Copy link
Member

frosch123 commented Jun 29, 2018 •
edited
Loading

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 TrueBrain added needs triage This issue needs further investigation before it becomes actionable bug Something isn't working labels Jul 12, 2018
@TrueBrain
Copy link
Member

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

@TrueBrain TrueBrain added waiting on author and removed needs triage This issue needs further investigation before it becomes actionable labels Aug 5, 2018
@bentley
Copy link
Author

bentley commented Aug 7, 2018

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

@TrueBrain TrueBrain added needs triage This issue needs further investigation before it becomes actionable and removed waiting on author labels Aug 11, 2018
@andythenorth andythenorth added the stale Stale issues label Jan 5, 2019
@andythenorth andythenorth removed the stale Stale issues label Jan 24, 2019
@stale
Copy link

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 Stale issues label Mar 25, 2019
@bentley
Copy link
Author

bentley commented Mar 26, 2019

This crash is still present in 1.9.0-RC1.

Sorry, something went wrong.

@stale stale bot removed the stale Stale issues label Mar 26, 2019
@LordAro
Copy link
Member

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

Sorry, something went wrong.

@bentley
Copy link
Author

bentley commented Jan 2, 2020 via email

Sorry, something went wrong.

@bentley
Copy link
Author

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.

Sorry, something went wrong.

@bentley bentley closed this as completed Jan 3, 2020
@LordAro
Copy link
Member

LordAro commented Jan 3, 2020

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

@LordAro
Copy link
Member

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
Assignees
No one assigned
Labels
bug Something isn't working needs triage This issue needs further investigation before it becomes actionable
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@bentley @TrueBrain @andythenorth @LordAro @frosch123