[DB] CLoginData memory leak

[Larry Watterson]

TL;DR: YMIR dev ağabeyler duplicate login'i temizleyeceğiz derken bir mekanizma kurmuşlar ama normal çıkışa bağlamayı unutup sıvamışlar.

oyuncumuz zühtü olsun;

zühtü oyuna giriş yapıyor. arka planda ne oluyor?

QUERY_AUTH_LOGIN çalışıyor, bir CLoginData nesnesi free store'dan allocate ediliyor. bu nesneye dört farklı container'dan non-owning raw pointer ile erişiliyor amma velakin "non-owning" demek aslında doğru değil(inanın böyle demeyi çok isterdim) çünkü codebasede ownership'i taşıyan tek tip yok, dördü de fiilen "owner" gibi davranıyor, gerçi bi refcount falan var ama dökümantasyon falan olmadığı için pek anlamlandıramadım, binevi elle shared_ptr taklit edilmiş işte danglinge düşmeyelin falan ama 10 saatimi kodu anlamaya çalışarak geçirecek değilim.

"o nasıl oluyor la?" derseniz non-static data memberları inceleyebilirsiniz:

1. m_map_pkLoginData // login-key -> nesne
2. m_map_pkLoginDataByLogin // "zuhtu123" -> nesne
3. m_map_pkLoginDataByAID // account id -> nesne
4. m_map_kLogonAccount // "zuhtu123" -> nesne

yani şu..., neyse(hocam dayanamıyorum YMIR'e laf atıcam).

diyelim ki zühtü biraz takılıp çıkıyor veya alt + f4 çekiyor. şimdi arka planda ne oluyor? QUERY_LOGOUT çalışıyor, orada da işte DeleteLogonAccount("zuhtu123") çağrılıyor.

DeleteLogonAccount'un flowu şöyle:

if (pkLD->IsDeleted())   // hımmmmmmmmmm
    delete pkLD;

m_map_kLogonAccount.erase(it);

"IsDeleted() nedir lo?" m_bDeleted non-static data memberı return eden bir accessor. bunu true set eden tek yer SetDeleted mutator, o da yalnızca DeleteLoginData() içinde çağrılıyor. DeleteLoginData()'yı çağıran tek yer ise QUERY_AUTH_LOGIN, yani bu demek oluyor ki aynı hesabın tekrar login olması gerekiyor.

yani düşünelim;

zühtü DB processi hiç restart olmayan bir sunucuda bir kez login/logout tetiklese ve bir daha hiç girmese ne olur? süper olurdu aslında ama, neyse olacak olan şu: QUERY_AUTH_LOGIN bir daha tetiklenmez, bu da sırayla şunlara yol açar:

• DeleteLoginData çağrılmaz
• SetDeleted çağrılmaz
• IsDeleted() hep false return eder
• "delete pkLD;" delete-expression hiçbir zaman çalışmaz

vay be zühtü, dağları devirdin.

ek olarak zühtü'den bağımsız aynı pattern RemovePeer() içinde de tekrar ediyor:

if (pkLD->IsDeleted())
{
    delete pkLD;
}

m_map_kLogonAccount.erase(it++);

bu sefer zühtü'nün suçu yok, bir game server peer'i koparsa (artık crash mi olur restart mı olur siz karar verin) o peer üzerinden bağlı tüm hesapları m_map_kLogonAccount'tan silmeye çalışlıyor ama yine aynı döngü IsDeleted normal şartlarda hep false.

yani game server bir sebepten ötürü restart attığı anda o sırada bağlı olan (kaç kişi olduğuna siz karar verin) oyuncuların CLoginData'sı free edilmiyor.

ee noldu yani derseniz olay tam olarak CWE-772(araştırın bi zahmet)

gözümden kaçmadıysa herhangi bir dangling pointer, use after free falan yok, diğer 3 containerda halen geçerli ve erişilebilir. olay nesnenin işi bitmiş ama halen serbest bırakılmamış olması. uğraştırıcı ve fark edilmesi yüksek ihtimal olsa da bir saldırı senaryosu kurulabilir. zaten normal oyuncu devride başlı başına bir dert, art niyetli birini aramanıza da gerek yok uptime'da yardımcı olur.

fix(naked pointer kullanmaya devam etmek isteyenler için):

    m_map_kLogonAccount.erase(it);

    if (pkLD->IsDeleted())
    {
        delete pkLD;
    }
    else
    {
        DeleteLoginData(pkLD);
    }

burda iteratoru önce erase etmezseniz "DeleteLoginData" çağrısının hiçbir anlamı kalmaz, önce erase edin. bu oyunun codebaseinin içine biraz dalmak bile insanı akıl sağlığından eder, bu büyük ihtimalle açtığım son konu.

rica ederim, iyi forumlar.

Not: warp akışını kontrol etmedim, bir sorun bariz olarak var ama direkt olarak delete etmek farklı bir duruma yol açabilir, Metin2 mayın tarlası gibi bir oyun. uygulayıp bir sorun yaşayan olursa belirtirse analizini yapıp düzeltiriz.

Özel rica: Lütfen kullandığınız AI chate girip "ne anlatıyor la bu? şuna bir yorum yaz kallavi olsun" benzeri bir şeyin çıktısıyla yorum yapmayın, bildiğiniz konu ise teknik konuşmak istiyor iseniz doğrusunu yanlışını konuşalım yoksa ne sizin tokenler gitsin ne ben yorulayım, teşekkürler.

 

[Uğur Nedely4]

Ellerine sağlık iyi calısmalarr

 

[Mehmetcan Y.k]

Paylaşım İçin Teşekkürler

 

[ftyazilimm]

DeleteLoginData fonksiyonuna bakarsak nesneyi sadece map'lerden temizliyor, delete etmiyor. Yani yazdığın else bloğu çalıştığında nesne her yerden silinip RAM'de ulaşılamaz şekilde sahipsiz kalıyor.
else kısmında DeleteLoginData(pkLD); altına bir de delete pkLD; eklersek sorun kusursuz çözülür bence zühtüde patlamaz
yinede eline sağlık paylaşım için teşekkürler

 

[Xorvex]

TL;DR: YMIR dev ağabeyler duplicate login'i temizleyeceğiz derken bir mekanizma kurmuşlar ama normal çıkışa bağlamayı unutup sıvamışlar.

oyuncumuz zühtü olsun;

zühtü oyuna giriş yapıyor. arka planda ne oluyor?

QUERY_AUTH_LOGIN çalışıyor, bir CLoginData nesnesi free store'dan allocate ediliyor. bu nesneye dört farklı container'dan non-owning raw pointer ile erişiliyor amma velakin "non-owning" demek aslında doğru değil(inanın böyle demeyi çok isterdim) çünkü codebasede ownership'i taşıyan tek tip yok, dördü de fiilen "owner" gibi davranıyor, gerçi bi refcount falan var ama dökümantasyon falan olmadığı için pek anlamlandıramadım, binevi elle shared_ptr taklit edilmiş işte danglinge düşmeyelin falan ama 10 saatimi kodu anlamaya çalışarak geçirecek değilim.

"o nasıl oluyor la?" derseniz non-static data memberları inceleyebilirsiniz:

1. m_map_pkLoginData // login-key -> nesne
2. m_map_pkLoginDataByLogin // "zuhtu123" -> nesne
3. m_map_pkLoginDataByAID // account id -> nesne
4. m_map_kLogonAccount // "zuhtu123" -> nesne

yani şu..., neyse(hocam dayanamıyorum YMIR'e laf atıcam).

diyelim ki zühtü biraz takılıp çıkıyor veya alt + f4 çekiyor. şimdi arka planda ne oluyor? QUERY_LOGOUT çalışıyor, orada da işte DeleteLogonAccount("zuhtu123") çağrılıyor.

DeleteLogonAccount'un flowu şöyle:

if (pkLD->IsDeleted())   // hımmmmmmmmmm
    delete pkLD;

m_map_kLogonAccount.erase(it);

"IsDeleted() nedir lo?" m_bDeleted non-static data memberı return eden bir accessor. bunu true set eden tek yer SetDeleted mutator, o da yalnızca DeleteLoginData() içinde çağrılıyor. DeleteLoginData()'yı çağıran tek yer ise QUERY_AUTH_LOGIN, yani bu demek oluyor ki aynı hesabın tekrar login olması gerekiyor.

yani düşünelim;

zühtü DB processi hiç restart olmayan bir sunucuda bir kez login/logout tetiklese ve bir daha hiç girmese ne olur? süper olurdu aslında ama, neyse olacak olan şu: QUERY_AUTH_LOGIN bir daha tetiklenmez, bu da sırayla şunlara yol açar:

• DeleteLoginData çağrılmaz
• SetDeleted çağrılmaz
• IsDeleted() hep false return eder
• "delete pkLD;" delete-expression hiçbir zaman çalışmaz

vay be zühtü, dağları devirdin.

ek olarak zühtü'den bağımsız aynı pattern RemovePeer() içinde de tekrar ediyor:

if (pkLD->IsDeleted())
{
    delete pkLD;
}

m_map_kLogonAccount.erase(it++);

bu sefer zühtü'nün suçu yok, bir game server peer'i koparsa (artık crash mi olur restart mı olur siz karar verin) o peer üzerinden bağlı tüm hesapları m_map_kLogonAccount'tan silmeye çalışlıyor ama yine aynı döngü IsDeleted normal şartlarda hep false.

yani game server bir sebepten ötürü restart attığı anda o sırada bağlı olan (kaç kişi olduğuna siz karar verin) oyuncuların CLoginData'sı free edilmiyor.

ee noldu yani derseniz olay tam olarak CWE-772(araştırın bi zahmet)

gözümden kaçmadıysa herhangi bir dangling pointer, use after free falan yok, diğer 3 containerda halen geçerli ve erişilebilir. olay nesnenin işi bitmiş ama halen serbest bırakılmamış olması. uğraştırıcı ve fark edilmesi yüksek ihtimal olsa da bir saldırı senaryosu kurulabilir. zaten normal oyuncu devride başlı başına bir dert, art niyetli birini aramanıza da gerek yok uptime'da yardımcı olur.

fix(naked pointer kullanmaya devam etmek isteyenler için):

    m_map_kLogonAccount.erase(it);

    if (pkLD->IsDeleted())
    {
        delete pkLD;
    }
    else
    {
        DeleteLoginData(pkLD);
    }

burda iteratoru önce erase etmezseniz "DeleteLoginData" çağrısının hiçbir anlamı kalmaz, önce erase edin. bu oyunun codebaseinin içine biraz dalmak bile insanı akıl sağlığından eder, bu büyük ihtimalle açtığım son konu.

rica ederim, iyi forumlar.

Özel rica: Lütfen kullandığınız AI chate girip "ne anlatıyor la bu? şuna bir yorum yaz kallavi olsun" benzeri bir şeyin çıktısıyla yorum yapmayın, bildiğiniz konu ise teknik konuşmak istiyor iseniz doğrusunu yanlışını konuşalım yoksa ne sizin tokenler gitsin ne ben yorulayım, teşekkürler.

bu forumda ismi değişmiş oynayan elemanın paylaşım için teşekkürler