Bug in server processing of OP_GMZoneRequest

[RangerDown]

I found a bug in the way the server processes a /zone command. (This doesn't apply to emu's #zone, just the /zone command you can use when your GM flag is on.)

In my example, /zone Natimbi would take me to Erudin instead.

Packetcollect output for both a /zone Erudnext and /zone Natimbi:

*** This packet was generated by /zone erudnext

OPCode 0x0185 (OP_GMZoneRequest) [Raw OPCode: 0x0185] [Size: 88]
[192.168.0.5:1312 -> 192.168.0.1:7999]
[CPU Tick Count: 5603453]
0: 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 | ................
16: 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 | ................
32: 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 | ................
48: 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 | ................
64: 18 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 | ................
80: 00 00 00 00 00 00 00 00 | ........

*** This packet was generated by /zone Natimbi

OPCode 0x0185 (OP_GMZoneRequest) [Raw OPCode: 0x0185] [Size: 88]
[192.168.0.5:1317 -> 192.168.0.1:7995]
[CPU Tick Count: 5616125]
0: 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 | ................
16: 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 | ................
32: 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 | ................
48: 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 | ................
64: 18 01 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00 | ................
80: 00 00 00 00 00 00 00 00 | ........



The cause of the problem pretty much jumps out at ya: the server's only looking at one byte (the 0x1 when it should be looking at 2 bytes. Evidently with the GoD zones, they started needing to use 2 bytes, and Natimbi happens to have its first byte as 0x18 too.

[RangerDown]

Update: I checked out the source code and I don't think the bug is with OP_GMZoneRequest, I think it's actually in the handling of OP_GMZoneRequest2 (opcode 0x023c)

This is the block of code in client_process.cpp that handles that:

case OP_GMZoneRequest2: {
int32 zonereq = (int32)*app->pBuffer;
if(zonereq == zone->GetZoneID())
this->MovePC(zonereq, zone->safe_x(), zone->safe_y(), zone->safe_z(), 0, false);
else
this->MovePC(zonereq, -1, -1, -1, 0, false);
break;
}

That first line:

int32 zonereq = (int32)*app->pBuffer;

just doesn't look right. I think it's evaluating *app->pBuffer to be only a 1-byte integer (pbuffer is type uchar after all), THEN casting that to int32 which doesn't do us much good. Before LDoN/GoD, we could get away with it because there were less than 256 zones.

Perhaps it should be changed to:

int32 zonereq = (int32) (app->pBuffer[0] + (app->pBuffer[1]*256));

I don't (yet) have a compiler or I'd test this out myself.

Course, even that suggested line is a bit short-sighted... I mean, what are we gonna do when there's more than 65,536 zones in the game? :P

[RangerDown]

Or:

int32 zonereq;
memcpy(&zonereq,app->pBuffer,sizeof(int32));