----------------------------------- A] buffer-overflow in recv_add_unit ----------------------------------- The command used for adding units (just the first command used at the beginning of the challenge) is affected by a buffer-overflow vulnerability which happens during the copying of the incoming data to the name buffer of only 26 bytes. From multiplay.cpp: int Net::recv_add_unit() { int num; char name[26]; int cost; pkt >> num; pkt >> name; ... -------------------------------------------- B] invalid memory access in decode_stringmap -------------------------------------------- When a packet is received the server calls decode_stringmap which is used for reading the number of informations (keys and values) contained in the incoming data block and for their subsequent reading. Here exist two problems: - invalid size values can lead to the reading of the unallocated memory after the packet and to the subsequent crash of the server (for example keysize says to read 100 bytes while the packet is only 2 bytes long) - the server terminates if keysize or valsize are too big and cannot be allocated with the resize function From server_transport.cpp: bool decode_stringmap(std::map<std::string, std::string> &info, const void *buffer) { const unsigned char *p = (const unsigned char *)buffer; unsigned int num = decode_unsigned_int(p); while (num--) { unsigned int keysize = decode_unsigned_int(p); unsigned int valsize = decode_unsigned_int(p); std::string key; key.resize(keysize); std::string val; val.resize(valsize); for (unsigned int i = 0; i < keysize; i++) key[i] = decode_unsigned_char(p); for (unsigned int i = 0; i < valsize; i++) val[i] = decode_unsigned_char(p); info[key] = val; } return true; } ----------------------------------------- C] possible code execution through arrays ----------------------------------------- Some commands can be used for crashing the remote client/opponent through invalid values (too big or negative) used for moving into the internal arrays of the game. Another effect is the possibility to execute malicious code, in fact the game uses large numbers (usually signed 32 bit values) which can be used to reach any location of the memory, then these commands allow the writing of the data contained in the packet into these locations like what happens with "pkt >> scenario->rules[index]" where our 32 bit number (pkt >>) is copied in the location chosed by us with index. These commands are recv_rules, recv_select_unit (select_unit checks only if num if major not minor), recv_options and recv_unit_data (with a negative value or minor than 19). From multiplay.cpp: int Net::recv_rules() { int index; pkt >> index; pkt >> scenario->rules[index]; ... ---------------- D] SQL injection ---------------- The server uses an internal SQL database for handling accounts and other informations about the matches. In the points where is used the user's input and the %s format argument instead of %q could be possible to inject own SQL commands in the query prepared by the server. From server_protocol.cpp: bool ServerClientUfo::recv_packet(NLuint id, const std::string &raw_packet) ... case SRV_GAME_REPLAY_REQUEST: { send_packet_back(SRV_GAME_RECOVERY_START, "1"); try { debug_game_id = atol(packet.c_str()); sqlite3::reader reader=db_conn.executereader("select command, packet_type, id from ufo2000_game_packets where game=%s order by id;", packet.c_str()); ... --------------------------------- E] mapdata global buffer overflow --------------------------------- mapdata is a global buffer declared in main.cpp as a GEODATA structure of 56 bytes which can be overflowed through the recv_map_data function. The effect is the immediate crash of the opponent. From multiplay.cpp: int Net::recv_map_data() { std::string map_name; std::string map_data; pkt >> map_name; pkt >> mapdata.x_size; pkt >> mapdata.y_size; pkt >> mapdata.z_size; pkt >> map_data; ASSERT((int)map_data.size() == mapdata.x_size * mapdata.y_size); memcpy(&mapdata.mapdata, map_data.data(), map_data.size()); .... http://aluigi.altervista.org/adv/ufo2ko-adv.txt
This one has been masked.
A fix is here: http://bugs.gentoo.org/show_bug.cgi?id=143421
Can I query status of this issue? Is there anything special needed to unmask ufo2000 in portage? Here is the history of this issue: 2006-07-10 E-mail from Luigi Auriemma with the security problems report. He got reply thanking him for this information and asking not to disclose this information until the bugfix gets released. 2006-07-13 Problems fixed in SVN, final pre-release testing in process. 2006-07-16 Security problem disclosed (without any notice to ufo2000 developers). 2006-07-18 Bugfix release 0.7.1062 available, links to vulnerable versions removed from download 2006-07-28 Somebody noticed security problem announcement and reported it in the forum: http://www.xcomufo.com/forums/index.php?showtopic=242026496 As a result, a special check preventing vulnerable versions from connecting to the server was added. E-mail asking to mask current vulnerable ufo2000 version in the portage was sent to Chris Gianelloni (the last gentoo developer who touched ufo2000 ebuild at that moment). 2006-07-31 ufo2000 0.6.627 was masked by Chris Gianelloni 2006-08-15 Ebuild for ufo2000 0.7.1062 added to portage (but still masked) 2006-11-27 Mask is still here...
0.6.627 removed and 0.7.1062 unmasked
flew under our radar it seems fixed version is in the tree for two months now Calling for a vote according to the policy...
I vote yes.
(In reply to comment #6) > I vote yes. > WHy not... ok, yes. GLSA Request filed.
Not everything is so easy as ufo2000 is currently masked because its optional dependency (dumb library 0.9.2) was removed from the portage tree, see http://bugs.gentoo.org/show_bug.cgi?id=164186 This dumb 0.9.2 had some security issue also discovered by Luigi Auriemma, the problem was solved in version 0.9.3 which is unfortunately API incompatible with 0.9.2. As ufo2000 uses dumbogg plugin (discontinued by upstream) which heavily relies on dumb internals, fixing it to work with a new version seems to be a nontrivial task. So support for dumb will be most likely dropped from the future versions of ufo2000 and replaced with something else. Right now the best option probably would be to fix ufo2000 ebuild to remove this optional dumb dependency, unmask it and have this security issue finally resolved.
i think this will take a lot of time, so we'll issue the GLSA telling that ufo2000 is masked and users should unmerge it due to security issues both in dumb-0.9.2 and in old ufo2000 ebuilds.
GLSA 200702-10 sent, without closing the bug, in the enhancement scope waiting for a better solution.
Created attachment 111209 [details] ufo2000-0.7.1062-r1.ebuild
I'm sorry for a bit late reply, I did not get e-mail notification after comment #9 for some reason. Actually removing DUMB dependency is quite trivial, ebuild is attached. Security impact of running ufo2000 with vulnerable version of DUMB library is extremely low. Unless the users can be convinced by some stranger to manually replace default game music soundtracks with something else, they are safe.
After the second thought, it will really take some more time to resolve. In order to remain in the portage tree, ufo2000 needs a stable release that would be security problems free and remain supported for a reasonable time (half a year at least). Beta versions do not suit this purpose well. It is unrealistic to catch up with the beta releases once ufo2000 is actively developed (and a new beta 0.7.1067 is already available). A better solution for beta versions is an upstream maintained portage overlay instead and binary builds for other linux distributions. I'll try to come up with some way to solve this problem within a week.
(In reply to comment #12) > Security impact of running ufo2000 with vulnerable version of DUMB library is > extremely low. Unless the users can be convinced by some stranger to manually > replace default game music soundtracks with something else, they are safe. You're right. > > A better solution for beta versions is an upstream maintained portage overlay > instead and binary builds for other linux distributions. > > I'll try to come up with some way to solve this problem within a week. > thanks a lot
It's gone. Please go ahead and close this out.
GLSA 200702-10
