Revisiting aluigi's bf2urlz

Whilst playing around with my Battlefield 2 master server replacement I had the brilliant idea to run the application I wrote for players as admin. Immediately I received backlash from the community telling me about bf2urlz and how it would be even more dangerous if the game ran as admin.

Some googling further I found this, Aluigi's advisory. So it appears that the game does not properly sanitize the url it receives from the server. Time to fire up the old and trusty Ida Pro.

I see a mention of a folder named LogoCache, so when searching for this string I found the following function.

char __thiscall sub_4C3120(dice::hfe::ServerLogoManager *this, int a2)
{
  p_httpRequest = &this->httpRequest;
  v23 = &this->httpRequest;
  if ( sub_59C400(&this->httpRequest) )
    return 0;
  std::string::string(v19);
  std::string::string(v20);
  this->pVMT + 24(this, a2, v19, v20);
  v4 = dword_9FFCA8 + 72(dword_9FFCA8);
  v5 = std::string::string(v22, v4);
  v6 = std::string::operator+=(v5, "\\LogoCache\\");
  std::string::string(v18, v6);
  std::string::~string(v22);
  std::string::operator+=(v18);
  std::string::string(v21, "@HOME@\\LogoCache\\");
  std::string::operator+=(v21);
  std::string::operator+=(v21);
  v7 = std::string::string(v22, v18);
  v8 = std::string::operator+=(v7);
  std::string::string(v17, v8);
  std::string::~string(v22);
  std::string::string(v14);
  if ( dword_9FFCA8 + 56(
         dword_9FFCA8,
         v21,
         v14,
         v20,
         v20,
         v19,
         v19) ) // if the logo already exists
  {
    // check how old it is, if recent then skip
    GetSystemTimeAsFileTime(&SystemTimeAsFileTime);
    v9 = (SystemTimeAsFileTime.dwLowDateTime + ((unsigned __int64)SystemTimeAsFileTime.dwHighDateTime << 32))
       / 0x861C46800i64
       - (v15 + ((unsigned __int64)v16 << 32)) / 0x861C46800i64;
    SystemTimeAsFileTime.dwHighDateTime = HIDWORD(v9);
    if ( v9 < 0x18 )
      goto LABEL_9;
    p_httpRequest = v23;
  }
  dword_9FFCA8 + 32(dword_9FFCA8, v21);
  v10 = std::string::string(v13, v18);
  v11 = std::string::operator+=(v10, "/dummy.txt");
  std::string::string(v22, v11);
  std::string::~string(v13);
  dword_9FFCA8 + 48(dword_9FFCA8, v22, 0, 0, 1);
  dword_9FFCA8 + 32(dword_9FFCA8, v22);
  if ( !sub_59C5B0(p_httpRequest, a2, (int)v17, 0) )
  {
    return 0;
  }
  return 1;
}

It seems to craft where the file has to be stored. Before calling a function within httpRequest. NOTE: I heavily beautified this code by hand, you may not get the same output when finding this function.

Now what does sub_59C5B0 do?

char __thiscall sub_59C5B0(dice::hfe::HttpRequest *this, std::string downloadUrl, std::string downloadPath, char a4)
{
  const char *fileName; // eax
  const char *downloadLink; // ecx
  int v7; // eax

  if ( this->isRunning )
    return 0;
  this->isRunning = 1;
  this->dword14 = 1;
  fileName = downloadPath.c_str();
  downloadLink = downloadUrl.c_str();
  v7 = ghttpSaveEx(
         downloadLink,
         fileName,
         byte_87FB54,
         0,
         0,
         a4 == 0,
         (int)dice::hfe::HttpRequest::onProgress,
         (int)dice::hfe::HttpRequest::onError,
         this);
  this->dword8 = v7;
  if ( v7 < 0 )
  {
    this->isRunning = 0;
    return 0;
  }
  if ( !this->isRunning )
  {
    sub_59BF40(v7);
    this->dword8 = -1;
  }
  return 1;
}

It calls into ghttpSaveEx, this is a gamespy http function. So the game uses gamespy’s HTTP handling underneath.

int __fastcall ghttpSaveEx(
        const char *URL,
        const char *fileName,
        const char *headers,
        int post,
        int throttle,
        int blocking,
        int progressCallback,
        int completedCallback,
        dice::hfe::HttpRequest *param)
{
  _DWORD *v11; // eax
  _DWORD *v12; // esi
  char *v13; // eax
  char *v15; // eax
  FILE *filePtr; // eax

  if ( !URL || !*URL || !fileName || !*fileName )
    return -1;
  if ( !ghiReferenceCount )
    ghttpStartup();
  v11 = ghiNewConnection();
  v12 = v11;
  if ( !v11 )
    return -1;
  v11[3] = 1;
  v13 = goastrdup(URL);
  v12[5] = v13;
  if ( !v13 )
    goto LABEL_9;
  if ( headers )
  {
    if ( *headers )
    {
      v15 = goastrdup(headers);
      v12[11] = v15;
      if ( !v15 )
        goto LABEL_9;
    }
  }
  v12[16] = progressCallback;
  v12[17] = completedCallback;
  v12[91] = post;
  v12[13] = blocking;
  v12[18] = param;
  v12[89] = throttle;
  if ( post )
  {
    if ( !ghiPostInitState((int)v12) )
      goto LABEL_9;
  }
  filePtr = fopen(fileName, "wb"); // <-- opens handle to file
  v12[12] = filePtr;
  if ( !filePtr )
  {
LABEL_9:
    ghiFreeConnection(v12);
    return -1;
  }
  if ( !blocking )
    return v12[1];
  while ( !ghiProcessConnection((int)v12) )
    msleep(0xAu);
  return 0;
}

Alright we found the function that opens the file. So the problem lies in the game passing the value, a simple fix for this would be hooking this function and not sending any request if .. is found in the path. That is exactly what I did.

GHTTPRequest __fastcall DetouredghttpSaveEx(const char* URL, const char* filename, const char* headers, GHTTPPost post, GHTTPBool throttle, GHTTPBool blocking, ghttpProgressCallback progressCallback, ghttpCompletedCallback completedCallback, void* param)
{
    std::string filePath(filename);

    if (filePath.find("..") != std::string::npos)
    {
        printf("Path traversal attempt found!\n");
        return 0;
    }

    return realghttpSaveEx(URL, filename, headers, post, throttle, blocking, progressCallback, completedCallback, param);
}

For the exact parameters I used the gamespy sdk which can be found on GitHub. And with that you can also find the patch for this issue on my GitHub.

It’s interesting to see such old exploits that never got a fix, and it’s very rewarding to fix them. Helping the community enjoy their favorite game safely in the modern age.