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

                 Summary: Client sent uninitialised data in
PACKET_PLAYER_ATTRIBUTE_CHUNK, and other stories
                 Project: Freeciv
            Submitted by: jtn
            Submitted on: Sat Nov 23 20:03:07 2013
                Category: client
                Severity: 2 - Minor
                Priority: 5 - Normal
                  Status: None
             Assigned to: None
        Originator Email: 
             Open/Closed: Open
                 Release: 2.4.0
         Discussion Lock: Any
        Operating System: Any
         Planned Release: 2.4.1,2.5.0,2.6.0

    _______________________________________________________

Details:

Valgrind spotted that the client sends uninitialised data to the server at
turn end, in PACKET_PLAYER_ATTRIBUTE_CHUNK. It overestimates how long the
serialised form of the attribute will be, and doesn't correct afterwards, so
the end of the attribute block is garbage.

This is probably harmless, since it's at the end and when it's eventually
deserialised the future client will ignore the garbage. Nevertheless, it
should be fixed. Valgrind spew at the bottom, for reference.

Other changes while I'm in there:
* attribute_hash used memcmp() to compare structures, so could in theory
return false negatives. Probably harmless on real platforms as the struct in
question (attr_key) consists of four consecutive ints.
* Some of the checks when deserialising are wrong and could misfire (possibly
losing CMA data unnecessarily? Haven't checked if this can happen in
practice.)
* Out-of-date comments.

2.4.0, default settings, two players both on the same team (although that
turns out not to be necessary). Both clients saw this message on turn done.
First turn:


==2965== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==2965==    at 0x628C2CC: send (send.c:33)
==2965==    by 0x577EED: fc_writesocket (netintf.c:170)
==2965==    by 0x4CE9A7: write_socket_data.isra.2 (connection.c:202)
==2965==    by 0x4CECF6: flush_connection_send_buffer_all (connection.c:232)
==2965==    by 0x4E4D17: send_attribute_block (packets.c:688)
==2965==    by 0x43B763: send_turn_done (client_main.c:691)
==2965==    by 0x4156E4: toplevel_key_press_handler (gui_main.c:606)
==2965==    by 0x65D0DD7: ??? (in
/usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0.2400.10)
==2965==    by 0x71FECA1: g_closure_invoke (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2965==    by 0x720FD70: ??? (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2965==    by 0x7217D4D: g_signal_emit_valist (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2965==    by 0x7218211: g_signal_emit (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2965==  Address 0x18d424da is 74 bytes inside a block of size 40,960
alloc'd
==2965==    at 0x4C2B6CD: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2965==    by 0x577C62: fc_real_malloc (mem.c:84)
==2965==    by 0x4CF029: new_socket_packet_buffer (connection.c:430)
==2965==    by 0x4CF439: connection_common_init (connection.c:590)
==2965==    by 0x440F6A: make_connection (clinet.c:276)
==2965==    by 0x4410F2: try_to_connect (clinet.c:246)
==2965==    by 0x4415B4: try_to_autoconnect (clinet.c:519)
==2965==    by 0x43C66D: real_timer_callback (client_main.c:987)
==2965==    by 0x416495: ui_main (gui_main.c:1671)
==2965==    by 0x43BC05: client_main (client_main.c:590)
==2965==    by 0x745F76C: (below main) (libc-start.c:226)


Next turn:


==2900== Conditional jump or move depends on uninitialised value(s)
==2900==    at 0x4C2DCD3: bcmp (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2900==    by 0x502E67: send_packet_player_attribute_chunk_100
(packets_gen.c:13986)
==2900==    by 0x4E4C9F: send_attribute_block (packets.c:685)
==2900==    by 0x43B763: send_turn_done (client_main.c:691)
==2900==    by 0x71FEEC9: ??? (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x7217710: g_signal_emit_valist (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x7218211: g_signal_emit (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x6528844: ??? (in
/usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0.2400.10)
==2900==    by 0x71FEEC9: ??? (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x7217710: g_signal_emit_valist (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x7218211: g_signal_emit (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x652766C: ??? (in
/usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0.2400.10)
==2900==
==2900== Conditional jump or move depends on uninitialised value(s)
==2900==    at 0x502E6A: send_packet_player_attribute_chunk_100
(packets_gen.c:13987)
==2900==    by 0x4E4C9F: send_attribute_block (packets.c:685)
==2900==    by 0x43B763: send_turn_done (client_main.c:691)
==2900==    by 0x71FEEC9: ??? (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x7217710: g_signal_emit_valist (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x7218211: g_signal_emit (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x6528844: ??? (in
/usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0.2400.10)
==2900==    by 0x71FEEC9: ??? (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x7217710: g_signal_emit_valist (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x7218211: g_signal_emit (in
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3200.4)
==2900==    by 0x652766C: ??? (in
/usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0.2400.10)
==2900==    by 0x65D0DD7: ??? (in
/usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so.0.2400.10)





    _______________________________________________________

Reply to this item at:

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

_______________________________________________
  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