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

Crashfix: maskname() buffer size #1435

Merged
merged 7 commits into from
Oct 9, 2023

Conversation

michaelortmann
Copy link
Member

@michaelortmann michaelortmann commented Mar 12, 2023

Found by: jack3?
Patch by: michaelortmann
Fixes: #1434

One-line summary:
Fix maskname() buffer size

Additional description (if needed):
Bug since eggdrop 1.9.0rc1 842ef44

Test cases demonstrating functionality (if applicable):

.console #test *
[01:34:45] tcl: builtin dcc call: *dcc:console -HQ 1 #test *
[01:34:45] #-HQ# console #test *
=================================================================
==479219==ERROR: AddressSanitizer: global-buffer-overflow on address 0x5616810a4333 at pc 0x561680c6de17 bp 0x7ffc36d10d00 sp 0x7ffc36d10cf0
WRITE of size 1 at 0x5616810a4333 thread T0
    #0 0x561680c6de16 in my_strcpy /home/michael/projects/eggdrop5/eggdrop/src/misc.c:188
    #1 0x561680c40fd1 in maskname /home/michael/projects/eggdrop5/eggdrop/src/flags.c:261
    #2 0x561680bd32ef in do_console /home/michael/projects/eggdrop5/eggdrop/src/cmds.c:751
    #3 0x561680bd466c in cmd_console /home/michael/projects/eggdrop5/eggdrop/src/cmds.c:782
    #4 0x561680cd0986 in builtin_dcc /home/michael/projects/eggdrop5/eggdrop/src/tclhash.c:678
    #5 0x561680ca5e05 in tcl_call_stringproc_cd /home/michael/projects/eggdrop5/eggdrop/src/tcl.c:334
    #6 0x7fdf4b47f61f in TclNRRunCallbacks (/usr/lib/libtcl8.6.so+0x4061f)
    #7 0x7fdf4b481664 in TclEvalEx (/usr/lib/libtcl8.6.so+0x42664)
    #8 0x7fdf4b481f26 in Tcl_EvalEx (/usr/lib/libtcl8.6.so+0x42f26)
    #9 0x7fdf4b481f49 in Tcl_Eval (/usr/lib/libtcl8.6.so+0x42f49)
    #10 0x7fdf4b48256f in Tcl_VarEvalVA (/usr/lib/libtcl8.6.so+0x4356f)
    #11 0x7fdf4b48264d in Tcl_VarEval (/usr/lib/libtcl8.6.so+0x4364d)
    #12 0x561680cd127b in trigger_bind /home/michael/projects/eggdrop5/eggdrop/src/tclhash.c:742
    #13 0x561680cd2896 in check_tcl_bind /home/michael/projects/eggdrop5/eggdrop/src/tclhash.c:942
    #14 0x561680cd310b in check_tcl_dcc /home/michael/projects/eggdrop5/eggdrop/src/tclhash.c:974
    #15 0x561680c0ce17 in dcc_chat /home/michael/projects/eggdrop5/eggdrop/src/dcc.c:1068
    #16 0x561680c63da6 in mainloop main.c:857
    #17 0x561680c67f89 in main main.c:1274
    #18 0x7fdf4a43c78f  (/usr/lib/libc.so.6+0x2378f)
    #19 0x7fdf4a43c849 in __libc_start_main (/usr/lib/libc.so.6+0x23849)
    #20 0x561680b71414 in _start (/home/michael/eggdrop/eggdrop-1.9.5+0x275414)

0x5616810a4333 is located 0 bytes to the right of global variable 's' defined in 'flags.c:206:15' (0x5616810a4220) of size 275
SUMMARY: AddressSanitizer: global-buffer-overflow /home/michael/projects/eggdrop5/eggdrop/src/misc.c:188 in my_strcpy
Shadow bytes around the buggy address:
  0x0ac35020c810: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac35020c820: f9 f9 f9 f9 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9
  0x0ac35020c830: 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 03
  0x0ac35020c840: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac35020c850: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0ac35020c860: 00 00 00 00 00 00[03]f9 f9 f9 f9 f9 00 00 00 00
  0x0ac35020c870: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac35020c880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac35020c890: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac35020c8a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ac35020c8b0: f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==479219==ABORTING

@michaelortmann michaelortmann changed the title Fix maskname() buffer size Crashfix: maskname() buffer size Mar 12, 2023
@attritionorg
Copy link

@michaelortmann Does this represent a potential remote buffer overflow vulnerability?

@michaelortmann
Copy link
Member Author

michaelortmann commented Mar 13, 2023

@michaelortmann Does this represent a potential remote buffer overflow vulnerability?

@attritionorg No. console flags are not shared between bots. Also this buffer overflow, as bad as it always is, is limited in size (6 bytes) and content (no user defined input). so exploiting this would really be a local DOS only. The user must be authorized / connected to the bot. As in #1434 it often wont crash the bot. The crash here was caused by additional compiler flags -fsanitize=address to better illustrate it. Thank you for keeping an eye on security related stuff!

@attritionorg
Copy link

Excellent, thanks for the quick response!

@vanosg vanosg added this to the v1.10 milestone Apr 4, 2023
@vanosg
Copy link
Member

vanosg commented Oct 7, 2023

Let's just make this 1024 and be done with it?

@michaelortmann
Copy link
Member Author

michaelortmann commented Oct 7, 2023

currently buf size is 281, so lets make it 512?

@michaelortmann

This comment was marked as outdated.

@michaelortmann
Copy link
Member Author

michaelortmann commented Oct 7, 2023

.status
[...]
Configured with: 'CFLAGS=-O1 -g3 -fstack-protector-all -fstack-clash-protection -fsanitize=address,undefined -Wformat-truncation=2 -D_FORTIFY_SOURCE=2'

.+chan #test
[19:51:16] tcl: builtin dcc call: *dcc:+chan -HQ 1 #test
[19:51:16] #-HQ# +chan #test

.console #test *
[19:51:23] tcl: builtin dcc call: *dcc:console -HQ 1 #test *
[19:51:23] #-HQ# console #test *
Set your console to #test: mpjkcoblrxsdwvtuhg12345678 (msgs, public, joins, kicks/modes, cmds, misc, bots, linked bot msgs, raw, files, server input, debug, wallops, server output, botnet incoming, botnet outgoing, incoming share traffic, outgoing share traffic, level 1, level 2, level 3, level 4, level 5, level 6, level 7, level 8).

.console #test -8
[19:51:26] tcl: builtin dcc call: *dcc:console -HQ 1 #test -8
[19:51:26] #-HQ# console #test -8
Set your console to #test: mpjkcoblrxsdwvtuhg1234567 (msgs, public, joins, kicks/modes, cmds, misc, bots, linked bot msgs, raw, files, server input, debug, wallops, server output, botnet incoming, botnet outgoing, incoming share traffic, outgoing share traffic, level 1, level 2, level 3, level 4, level 5, level 6, level 7).

.console #test -*
[19:51:29] tcl: builtin dcc call: *dcc:console -HQ 1 #test -*
Set your console to #test: - (none).

.strip +*
[19:57:01] tcl: builtin dcc call: *dcc:strip -HQ 1 +*
[19:57:01] #-HQ# strip +*
Your strip settings are: cbruagoi (color, bold, reverse, underline, ansi, bells, ordinary, italics).

.strip -i
[19:57:04] tcl: builtin dcc call: *dcc:strip -HQ 1 -i
[19:57:04] #-HQ# strip -i
Your strip settings are: cbruago (color, bold, reverse, underline, ansi, bells, ordinary).

.strip -* 
[19:57:08] tcl: builtin dcc call: *dcc:strip -HQ 1 -*
[19:57:08] #-HQ# strip -*
Your strip settings are: - (none).

@michaelortmann
Copy link
Member Author

Also while cleaning up similar code in eggdrop i found and fixed a bug regarding status report for channel setting revengebot.

Test:

.chanset #test +revenge -revengebot
[...]
.chaninfo #test
[...]
     +protectops     -protectfriends +revenge        -revengebot

Before:

.status all
[...]
  Module: channels, v 1.2
    #test               : (not on channel)
      Options: dynamicbans userbans greet protectops dontkickops revenge revengebot shared dynamic cycle dynamicexempts userexempts dynamicinvites userinvites 

After:

.status all
[...]
  Module: channels, v 1.3
    #test               : (not on channel)
      Options: dynamicbans userbans greet protectops dontkickops revenge shared dynamic cycle dynamicexempts userexempts dynamicinvites userinvite

@vanosg vanosg merged commit 322bddb into eggheads:develop Oct 9, 2023
1 check passed
@michaelortmann michaelortmann deleted the fix.maskname branch October 9, 2023 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash after console changes
3 participants