URL:
  <http://gna.org/bugs/?21558>

                 Summary: Server crash in improvement_name_translation()
called from diplomat_sabotage()
                 Project: Freeciv
            Submitted by: jtn
            Submitted on: Wed Jan 29 22:08:57 2014
                Category: None
                Severity: 3 - Normal
                Priority: 5 - Normal
                  Status: None
             Assigned to: None
        Originator Email: 
             Open/Closed: Open
                 Release: between 2.3.2 and 2.3.3, probably
         Discussion Lock: Any
        Operating System: None
         Planned Release: 

    _______________________________________________________

Details:

Reported by akfaew in Longturn LT32 game. (Possibly this has happened 4x, but
we only have a coredump from the last one.)

Server is somewhere between 2.3.2 and 2.3.3 (reports itself as "2.3.2+"; "this
game has been running for half a year now"), probably with Longturn patches,
but I think it's unlikely to be important. LT32 ruleset (which is close to
civ2-3 but with 3x movement, approximately).

Segfault in:


const char *improvement_name_translation(const struct impr_type *pimprove)
{
  return name_translation(&pimprove->name);
}


Backtrace:


#0  improvement_name_translation (pimprove=0x0) at improvement.c:187
#1  0x0000000000467edb in diplomat_sabotage (pplayer=0x7c3d950,
pdiplomat=0xe3a4ca0, pcity=0xe7997a0, improvement=199) at diplomats.c:954
#2  0x000000000046df5d in server_handle_packet (type=<optimized out>,
packet=<optimized out>, pplayer=0x7fe351d90fe0, pconn=0x8a9d40) at
hand_gen.c:247
#3  0x00000000004088d4 in server_packet_input (pconn=0x8336e8,
packet=0x7fffffda, type=84) at srv_main.c:1669
#4  0x00000000004985b5 in incoming_client_packets (pconn=<optimized out>) at
sernet.c:449
#5  server_sniff_all_input () at sernet.c:826
#6  0x000000000040a5ed in srv_running () at srv_main.c:2293
#7  0x000000000040b3a1 in srv_main () at srv_main.c:2699
#8  0x0000000000403cc4 in main (argc=15, argv=0x7fff23aaf568) at
civserver.c:377


----

My analysis so far (based on 2.3.3 source code and hoping that's close
enough):

Improvement ID 199 is not >= B_LAST (200) so it goes down the "Told which
improvement to pick" path in diplomat_sabotage() (rather than the "pick random
target" path). But 199 is presumably invalid in this ruleset so
improvement_by_number() returns NULL; city_has_building() silently returns
FALSE in this case; we try to print "Your <Spy> could not find the %s to
sabotage in <city>"; improvement_name_translation() goes kaboom when passed
NULL.

Clearly the server should be doing better validation on what it receives from
the client here. I haven't worked out whether there's a separate bug in any
released S2_3 client that can send 199 by mistake.

199 is suspiciously B_LAST-1, which is extra suspicious when you see that
there's a random +1/-1 in the value sent over the network; plenty of scope for
off-by-one errors.

More later, unless someone else gets there first.




    _______________________________________________________

Reply to this item at:

  <http://gna.org/bugs/?21558>

_______________________________________________
  Message sent via/by Gna!
  http://gna.org/


_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to