Author: sveinung
Date: Sun Oct 11 17:13:06 2015
New Revision: 30044
URL: http://svn.gna.org/viewcvs/freeciv?rev=30044&view=rev
Log:
order system: avoid unsafe is_valid_dir() usage
If a direction is valid given certain game settings was stored in an array
in patch #6325. The function is_valid_dir() would then look it up. This
doesn't work for direction values outside its range.
Don't use is_valid_dir() when validating the direction of unit orders sent
by the client over the network. Don't use it to check if the target is
origin tile or an adjacent tile.
See bug #23926
Modified:
trunk/common/map.c
trunk/common/map.h
trunk/server/unithand.c
trunk/server/unittools.c
Modified: trunk/common/map.c
URL:
http://svn.gna.org/viewcvs/freeciv/trunk/common/map.c?rev=30044&r1=30043&r2=30044&view=diff
==============================================================================
--- trunk/common/map.c (original)
+++ trunk/common/map.c Sun Oct 11 17:13:06 2015
@@ -1213,12 +1213,31 @@
/**************************************************************************
Returns TRUE iff the given direction is a valid one.
+
+ If the direction could be out of range you should use
+ map_untrusted_dir_is_valid() in stead.
**************************************************************************/
bool is_valid_dir(enum direction8 dir)
{
fc_assert_ret_val(direction8_is_valid(dir), FALSE);
return dir_validity[dir];
+}
+
+/**************************************************************************
+ Returns TRUE iff the given direction is a valid one.
+
+ Doesn't trust the input. Can be used to validate a direction from an
+ untrusted source.
+**************************************************************************/
+bool map_untrusted_dir_is_valid(enum direction8 dir)
+{
+ if (!direction8_is_valid(dir)) {
+ /* Isn't even in range of direction8. */
+ return FALSE;
+ }
+
+ return is_valid_dir(dir);
}
/**************************************************************************
Modified: trunk/common/map.h
URL:
http://svn.gna.org/viewcvs/freeciv/trunk/common/map.h?rev=30044&r1=30043&r2=30044&view=diff
==============================================================================
--- trunk/common/map.h (original)
+++ trunk/common/map.h Sun Oct 11 17:13:06 2015
@@ -597,6 +597,7 @@
enum direction8 dir_cw(enum direction8 dir);
enum direction8 dir_ccw(enum direction8 dir);
const char* dir_get_name(enum direction8 dir);
+bool map_untrusted_dir_is_valid(enum direction8 dir);
bool is_valid_dir(enum direction8 dir);
bool is_cardinal_dir(enum direction8 dir);
Modified: trunk/server/unithand.c
URL:
http://svn.gna.org/viewcvs/freeciv/trunk/server/unithand.c?rev=30044&r1=30043&r2=30044&view=diff
==============================================================================
--- trunk/server/unithand.c (original)
+++ trunk/server/unithand.c Sun Oct 11 17:13:06 2015
@@ -3818,7 +3818,7 @@
switch (packet->orders[i]) {
case ORDER_MOVE:
case ORDER_ACTION_MOVE:
- if (!is_valid_dir(packet->dir[i])) {
+ if (!map_untrusted_dir_is_valid(packet->dir[i])) {
log_error("handle_unit_orders() %d isn't a valid move direction. "
"Sent in order number %d from %s to unit number %d.",
packet->dir[i], i,
@@ -3921,7 +3921,7 @@
/* Validate individual actions. */
switch ((enum gen_action) packet->action[i]) {
case ACTION_FOUND_CITY:
- if (is_valid_dir(packet->dir[i])) {
+ if (map_untrusted_dir_is_valid(packet->dir[i])) {
/* Actor must be on the target tile. */
log_error("handle_unit_orders() can't do %s to a neighbor tile. "
"Sent in order number %d from %s to unit number %d.",
Modified: trunk/server/unittools.c
URL:
http://svn.gna.org/viewcvs/freeciv/trunk/server/unittools.c?rev=30044&r1=30043&r2=30044&view=diff
==============================================================================
--- trunk/server/unittools.c (original)
+++ trunk/server/unittools.c Sun Oct 11 17:13:06 2015
@@ -4056,7 +4056,7 @@
case ORDER_PERFORM_ACTION:
log_debug(" orders: doing action %d", order.action);
- if (!is_valid_dir(order.dir)) {
+ if (!direction8_is_valid(order.dir)) {
/* The target of the action is on the actor's tile. */
dst_tile = unit_tile(punit);
} else {
_______________________________________________
Freeciv-commits mailing list
Freeciv-commits@gna.org
https://mail.gna.org/listinfo/freeciv-commits
