[CF-Devel] Some bug fixes

[crossfire-devel at archives.real-time.com]

Mark Wedel wrote:
>
     
      Andreas Kirschbaum wrote:
     
     >
     
      > The patch patch-3.diff fixes two out of range errors in lines 737+
     
     >
     
      > and 1061+. The remaining changes replace the sizeof-expressions with
     
     >
     
      > the macro "arraysize". 
     
     >
     
     
     >
     
      If not seen any problems here, so I'm not positive what that change is
     
     >
     
      fixing.
     
     
This change is fixing accesses to max_titles[] for incorrect indices
(i.e. book types). My server crashed because the bookarch file did
contain the types 20, 30, 40, 50, and 100 besides the (correct) types
0-5.

While loading the file, the function init_book_archive() in
common/readable.c does not check the values for "type", so the variable
"booklist" gets 101 entries.

The loop in line 737+ then traverses this list and prints the
corresponding values from max_titles[]. This does not work because
max_titles has 6 entries only.

The other fix in line 1061+ triggers when a book with an incorrect type
value is created, i.e. change_book() is called with with an incorrect
msgtype value.


To reproduce the problem, I deleted my invalid bookarch file and put an
assert(number < 6) into get_titlelist(). It triggered in map
/wolfsburg/eeur/tower1.4 with type=30. The stack trace (including my
annotations) was:

#0  0x400b4571 in kill () from /lib/libc.so.6
#1  0x400b4315 in raise () from /lib/libc.so.6
#2  0x400b5838 in abort () from /lib/libc.so.6
#3  0x400ada4f in __assert_fail () from /lib/libc.so.6
#4  0x082169ac in get_titlelist (i=30) at readable.c:520
#5  0x08219411 in change_book (book=0x911180c, msgtype=30) at
    readable.c:1057

    book->name = "scroll"
    book->msg = "\n <undecipherable text>\n"

#6  0x0821d918 in tailor_readable_ob (book=0x4, msg_type=30) at
    readable.c:1961

    book is a register variable, so the debugger could not retrieve the
    correct value.

#7  0x0822b4f0 in fix_generated_item (op=0x4, creator=0x911266c,
    difficulty=152115212, max_magic=0, flags=8) at treasure.c:904

    op and difficulty are register variables.

    treasure.c:904: tailor_readable_ob(op,(creator&&creator->stats.sp)?creator->stats.sp:-1);

    creator->name = "demon"
    creator->stats.sp = 30

#8  0x08228c12 in create_one_treasure (tl=0x86ba460, op=0x911266c,
    flag=8, difficulty=4, tries=6) at treasure.c:388
#9  0x08228d96 in create_treasure (t=0x86ba460, op=0x911266c, flag=8,
    difficulty=4, tries=5) at treasure.c:410
#10 0x08228a72 in create_one_treasure (tl=0x86ba480, op=0x911266c,
    flag=8, difficulty=4, tries=4) at treasure.c:379
#11 0x08228d96 in create_treasure (t=0x86ba480, op=0x911266c, flag=8,
    difficulty=4, tries=3) at treasure.c:410
#12 0x08228504 in create_all_treasures (t=0x4, op=0x911266c, flag=8,
    difficulty=4, tries=2) at treasure.c:335
#13 0x08228413 in create_all_treasures (t=0x4, op=0x911266c, flag=8,
    difficulty=4, tries=2) at treasure.c:353

#14..#20 are the same as #13

#21 0x08228de5 in create_treasure (t=0x86ba480, op=0x911266c, flag=8,
    difficulty=4, tries=3) at treasure.c:412
#22 0x08228504 in create_all_treasures (t=0x4, op=0x911266c, flag=8,
    difficulty=4, tries=1) at treasure.c:335
#23 0x08228413 in create_all_treasures (t=0x4, op=0x911266c, flag=8,
    difficulty=4, tries=1) at treasure.c:353
#24 0x08228de5 in create_treasure (t=0x86ba480, op=0x911266c, flag=8,
    difficulty=4, tries=3) at treasure.c:412
#25 0x0816126f in generate_monster_arch (gen=0x9120890) at time.c:158

    server/time.c:157: if(HAS_RANDOM_ITEMS(op))
    server/time.c:158:     create_treasure(op->randomitems,op,GT_APPLY,
    server/time.c:159:         gen->map->difficulty,0);

    gen->name = "generator"
    gen->race = "demon"

#26 0x08161448 in generate_monster (gen=0x3) at time.c:174
#27 0x0816cd8b in process_object (op=0x4) at time.c:1266
#28 0x080da478 in process_events (map=0x0) at main.c:965
#29 0x080db909 in main (argc=1, argv=0xbffff734) at main.c:1183

After that, I think the root cause is that "demon"->stats.sp is used as
the book type. Because I do not know the code well enough, I can't
suggest a fix.

Nevertheless, I still think the functions init_book_archive() and
change_book() should check the (external) values they use.


Andreas

_______________________________________________
crossfire-devel mailing list
     
     crossfire-devel at lists.real-time.com
     
     
     https://mailman.real-time.com/mailman/listinfo/crossfire-devel