[Freeciv-Dev] Re: (PR#9686) technology transfer = server crash

["Marko Lindqvist" <marko.lindqvist@xxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=9686 >

Per I. Mathisen wrote:
> 
> Ah, more importantly, which client did you use? Not every client is
> updated after the tech change which made the server send A_UNSET for techs
> when you no longer have a tech goal for choosing free techs.

  But it was server that crashed. Attached patch prevents malicious 
clients from crashing server by sending illegal tech id's, but it cannot 
be casue to this crash. It would assert way earlier.

  If I get this logic right in my quick and sleepy glance, research 
might stay as A_UNSET until client explicitly sets it. So client can be 
'malicious' just by not sending a packet, for one reason or other 
(disconnected player). How would Darwin work with this scheme... wait, I 
just saw bug raport about Darwin. When Darwin gives its first tech, 
research is set to A_UNSET and when we are handling second free tech, 
assert 'researching != A_UNSET' trigggers.

  Certainly this assert should be removed and if no tech is selected 
before one should be gained, just random tech is gained.


  - Caz


diff -Nurd freeciv/server/plrhand.c freeciv/server/plrhand.c
--- freeciv/server/plrhand.c    2004-08-12 18:04:49.500000000 +0300
+++ freeciv/server/plrhand.c    2004-08-12 22:32:44.875000000 +0300
@@ -806,6 +806,12 @@
 **************************************************************************/
 void handle_player_research(struct player *pplayer, int tech)
 {
+  if (tech < A_FIRST || tech >= A_LAST_REAL) {
+    freelog(LOG_ERROR,
+            "packet_player_research from %s contained illegal tech id %d.",
+            pplayer->name, tech);
+    return;
+  }
   choose_tech(pplayer, tech);
   send_player_info(pplayer, pplayer);