Core/Tickets: Fix memory leak when malformed CMSG_GMTICKET_CREATE is received

src/server/game/Handlers/TicketHandler.cpp

@@ -49,13 +49,23 @@ void WorldSession::HandleGMTicketCreateOpcode(WorldPacket& recvData)
     // Player must not have ticket
     if (!ticket || ticket->IsClosed())
     {
-        ticket = new GmTicket(GetPlayer(), recvData);
-
+        uint32 mapId;
+        float x, y, z;
+        std::string message;
+        uint32 needResponse;
+        bool needMoreHelp;
         uint32 count;
         std::list<uint32> times;
         uint32 decompressedSize;
         std::string chatLog;
 
+        recvData >> mapId;
+        recvData >> x >> y >> z;
+        recvData >> message;
+
+        recvData >> needResponse;
+        recvData >> needMoreHelp;
+
         recvData >> count;
 
         for (uint32 i = 0; i < count; i++)
@@ -77,19 +87,25 @@ void WorldSession::HandleGMTicketCreateOpcode(WorldPacket& recvData)
             if (uncompress(dest.contents(), &realSize, recvData.contents() + pos, recvData.size() - pos) == Z_OK)
             {
                 dest >> chatLog;
-                ticket->SetChatLog(times, chatLog);
             }
             else
             {
                 TC_LOG_ERROR("network", "CMSG_GMTICKET_CREATE possibly corrupt. Uncompression failed.");
                 recvData.rfinish();
-                delete ticket;
                 return;
             }
 
             recvData.rfinish(); // Will still have compressed data in buffer.
         }
 
+        ticket = new GmTicket(GetPlayer());
+        ticket->SetPosition(mapId, x, y, z);
+        ticket->SetMessage(message);
+        ticket->SetGmAction(needResponse, needMoreHelp);
+
+        if (!chatLog.empty())
+            ticket->SetChatLog(times, chatLog);
+
         sTicketMgr->AddTicket(ticket);
         sTicketMgr->UpdateLastChange();
 

src/server/game/Tickets/TicketMgr.cpp

@@ -34,26 +34,13 @@ inline float GetAge(uint64 t) { return float(time(NULL) - t) / DAY; }
 // GM ticket
 GmTicket::GmTicket() : _id(0), _playerGuid(0), _posX(0), _posY(0), _posZ(0), _mapId(0), _createTime(0), _lastModifiedTime(0),
                        _closedBy(0), _assignedTo(0), _completed(false), _escalatedStatus(TICKET_UNASSIGNED), _viewed(false),
-                       _needResponse(false), _haveTicket(false) { }
+                       _needResponse(false), _needMoreHelp(false) { }
 
-GmTicket::GmTicket(Player* player, WorldPacket& recvData) : _createTime(time(NULL)), _lastModifiedTime(time(NULL)), _closedBy(0), _assignedTo(0), _completed(false), _escalatedStatus(TICKET_UNASSIGNED), _viewed(false), _haveTicket(false)
+GmTicket::GmTicket(Player* player) : _createTime(time(NULL)), _lastModifiedTime(time(NULL)), _closedBy(0), _assignedTo(0), _completed(false), _escalatedStatus(TICKET_UNASSIGNED), _viewed(false), _needMoreHelp(false)
 {
     _id = sTicketMgr->GenerateTicketId();
     _playerName = player->GetName();
     _playerGuid = player->GetGUID();
-
-    uint32 mapId;
-    recvData >> mapId;                      // Map is sent as UInt32!
-    _mapId = mapId;
-
-    recvData >> _posX;
-    recvData >> _posY;
-    recvData >> _posZ;
-    recvData >> _message;
-    uint32 needResponse;
-    recvData >> needResponse;
-    _needResponse = (needResponse == 17);   // Requires GM response. 17 = true, 1 = false (17 is default)
-    recvData >> _haveTicket;                // Requests further GM interaction on a ticket to which a GM has already responded. Basically means "has a new ticket"
 }
 
 GmTicket::~GmTicket() { }
@@ -80,7 +67,7 @@ bool GmTicket::LoadFromDB(Field* fields)
     _completed          = fields[++index].GetBool();
     _escalatedStatus    = GMTicketEscalationStatus(fields[++index].GetUInt8());
     _viewed             = fields[++index].GetBool();
-    _haveTicket         = fields[++index].GetBool();
+    _needMoreHelp       = fields[++index].GetBool();
     return true;
 }
 
@@ -107,7 +94,7 @@ void GmTicket::SaveToDB(SQLTransaction& trans) const
     stmt->setBool  (++index, _completed);
     stmt->setUInt8 (++index, uint8(_escalatedStatus));
     stmt->setBool  (++index, _viewed);
-    stmt->setBool  (++index, _haveTicket);
+    stmt->setBool  (++index, _needMoreHelp);
 
     CharacterDatabase.ExecuteOrAppend(trans, stmt);
 }
@@ -124,7 +111,7 @@ void GmTicket::WritePacket(WorldPacket& data) const
     data << uint32(GMTICKET_STATUS_HASTEXT);
     data << uint32(_id);
     data << _message;
-    data << uint8(_haveTicket);
+    data << uint8(_needMoreHelp);
     data << GetAge(_lastModifiedTime);
     if (GmTicket* ticket = sTicketMgr->GetOldestOpenTicket())
         data << GetAge(ticket->GetLastModifiedTime());
@@ -220,6 +207,20 @@ void GmTicket::SetUnassigned()
     }
 }
 
+void GmTicket::SetPosition(uint32 mapId, float x, float y, float z)
+{
+    _mapId = mapId;
+    _posX = x;
+    _posY = y;
+    _posZ = z;
+}
+
+void GmTicket::SetGmAction(uint32 needResponse, bool needMoreHelp)
+{
+    _needResponse = (needResponse == 17);   // Requires GM response. 17 = true, 1 = false (17 is default)
+    _needMoreHelp = needMoreHelp;           // Requests further GM interaction on a ticket to which a GM has already responded. Basically means "has a new ticket"
+}
+
 void GmTicket::TeleportTo(Player* player) const
 {
     player->TeleportTo(_mapId, _posX, _posY, _posZ, 0.0f, 0);

src/server/game/Tickets/TicketMgr.h

@@ -82,7 +82,7 @@ class GmTicket
 {
 public:
     GmTicket();
-    GmTicket(Player* player, WorldPacket& recvData);
+    GmTicket(Player* player);
     ~GmTicket();
 
     bool IsClosed() const { return _closedBy; }
@@ -129,6 +129,8 @@ public:
     void SetComment(std::string const& comment) { _comment = comment; }
     void SetViewed() { _viewed = true; }
     void SetUnassigned();
+    void SetPosition(uint32 mapId, float x, float y, float z);
+    void SetGmAction(uint32 needResponse, bool needMoreHelp);
 
     void AppendResponse(std::string const& response) { _response += response; }
 
@@ -164,7 +166,7 @@ private:
     GMTicketEscalationStatus _escalatedStatus;
     bool _viewed;
     bool _needResponse; /// @todo find out the use of this, and then store it in DB
-    bool _haveTicket;
+    bool _needMoreHelp;
     std::string _response;
     std::string _chatLog; // No need to store in db, will be refreshed every session client side
 };