详解银狐远控源码中那些C++编码问题

特别申明：

本文内容仅限于用作技术交流，请勿使用本文介绍的技术做任何其他用途，否则后果自负，与本号无关。

原始的银狐远程控制软件中，存在大量C++编码问题，大多数错误都属于低级错误。这些错误造成银狐远控稳定性差，容易崩溃。

本次代码以4.0为版本，Visual Studio 2010工程，希望对小伙伴们提高C/C++开发水平有帮助。

问题1

如图所示，银狐代码中有大量调用Windows API GetModuleFileName获取当前程序所在路径，该函数的最后一个参数传递的是字符串数量，不是字符串字节数目。由于工程设置使用了Unicode字符集合，一个字符占两个字节，而图中传递的是字节数目，当存放路径较长时，容易造成内存越界，引起崩溃。

DWORD GetModuleFileNameW( [in, optional] HMODULE hModule, [out] LPWSTR lpFilename, [in] DWORD nSize // 这个参数是字符数量，不是字节数目 );

修改方法：

将第三个参数改成字符数量，Windows定义了一个宏ARRAYSIZE可以编译器期间自动计算字符数据长度，其实现是数组总字节数除以第一个元素的字节数，即为字符长度。

将上述代码改成：

TCHAR ExePath[MAX_PATH] = { 0 }; GetModuleFileName(NULL, ExePath, ARRAYSIZE(ExePath));

这样的代码，在银狐代码中至少存在十几处，尤其是主控代码中。

问题2

如图所示，这里节选的是系统管理插件的代码，不知道读者能否看出标红处的问题？

这里使用memcpy将一个空字符拷贝至lpBuffer + dwOffset所在位置的内存处，拷贝长度是MAX_PATH * sizeof(TCHAR)。这里的行为是未定义的，由于宽字符_T("")指向的内存值就是L\0，这里拷贝长度过多，造成内存越界，行为未知。

修改方法：

作者的原意是将MAX_PATH * sizeof(TCHAR)长度的内存拷贝过去，所以这里可以简单改成使用memset函数做清零就可以了。

memset(lpBuffer + dwOffset, 0, MAX_PATH * sizeof(TCHAR));

这样的代码到处都是，尤其是在系统管理插件模块。

问题3

如图所示，问题所属模块为登录模块离线键盘记录功能。聪明的读者能看出这段代码的问题吗？

问题如下：

GlobalSize这个API返回的就是当前剪贴板数据的字节数，即使剪贴板内容是宽字符，所以计算字节内容时不该乘以2，现在乘以2可能造成接下来的wcscmp(lpstr, Clipboard_old) != 0代码中比较时读取lpstr越界。
同理memcpy(Clipboard_old, lpstr, nPacketLen);中读取lpstr也可能越界。
使用wsprintf(temp, _T("\r\n[剪切板:]%s\r\n"), lpstr);格式化字符时，由于没有指定temp最大缓冲区长度，那么格式化lpstr时会一直往后读取直到\0为止，问题是，lpstr不一定会以\0结束，所以可能一直往后读，此时内存已经越界。应该改写指定缓冲区最大长度的格式化函数来格式化剪贴板内容。

真是10行代码，三个崩溃。。。。。。

完整修复后的代码如下：

if (GetTickCount() - m_dwLastCapture > 1500) { InterlockedExchange((LPLONG)&m_dwLastCapture, GetTickCount()); OpenClipboard(NULL); HGLOBAL hglb = GetClipboardData(CF_UNICODETEXT); if (hglb != NULL) { //int nPacketLen = int(GlobalSize(hglb)) * 2 + 2; int nPacketLen = int(GlobalSize(hglb)); LPCTSTR lpstr = (LPCTSTR)GlobalLock(hglb); if (lpstr != NULL) { if (nPacketLen < sizeof(szClipboard_old)) // 判断长度 { //if (wcscmp(lpstr, szClipboard_old) != 0) //判断内容 if (memcmp((char*)lpstr, (char*)szClipboard_old, nPacketLen) != 0) { memcpy(szClipboard_old, lpstr, nPacketLen); //wsprintf(szTemp, _T("\r\n[剪切板:]%s\r\n"), lpstr); swprintf_s(szTemp, ARRAYSIZE(szTemp), _T("\r\n[剪切板:]%s\r\n"), lpstr); Input::SaveToFile(szTemp); memset(szTemp, 0, sizeof(szTemp)); } } } ::GlobalUnlock(hglb); } ::CloseClipboard(); }

问题4

如图所示，问题还是位于模块为登录模块离线键盘记录功能。

这段代码获取被控机器上当前活动窗口的窗口标题，同问题一，GetWindowText最后一个参数应该传入缓冲区字符数量（最后一个L'\0'）也要计算在内，这里传入了缓冲区字节数目，多了一倍，如果用户的电脑上活动窗口标题比较长，会造成内存溢出，被控崩溃。

修改方法：

BOOL Input::IsWindowsFocusChange() { memset(WindowCaption, 0, sizeof(WindowCaption)); hFocus = GetForegroundWindow(); GetWindowText(hFocus, WindowCaption, ARRAYSIZE(WindowCaption)); WindowCaption[ARRAYSIZE(WindowCaption)-1] = 0; //... 省略无关代码 }

下面的：

if (lstrlen(WindowCaption) > 0) { SYSTEMTIME s; GetLocalTime(&s); wsprintf(temp, _T("\r\n[标题:]%s\r\n[时间:]%d-%d-%d %d:%d:%d\r\n"), WindowCaption, s.wYear, s.wMonth, s.wDay, s.wHour, s.wMinute, s.wSecond); SaveToFile(temp); memset(temp, 0, sizeof(temp)); memset(WindowCaption, 0, sizeof(WindowCaption)); ReturnFlag = TRUE; }

调用wsprintf格式化存在与问题三一样的问题，当活动窗口标题较长时，内存也存在越界风险。

这也就解释了为什么当被控电脑点击不同窗口时，有时候被控会掉线（已经崩溃闪退）。

问题5

如图所示，代码位于主控远程屏幕相关代码。这里的代码也存在内存问题，但是如果不熟悉相关的Windows API，可能无法解决。

问题现象是启动远程屏幕主控偶现崩溃。

这个问题我用Visual Studio 2022去排查的，因为高版本的VS集成了Google Address Sanitizer，可以很方便定位C/C++内存问题。具体方法可以看这里。

解决方法：

查阅了一下 MSDN，发现GlobalAlloc函数分配内存时可以指定标志位，当标志位为GMEM_MOVEABLE时，分配的内存为可移动内存，CreateStreamOnHGlobal函数的第二个参数如果指定为 TRUE 时，在调用IStream::Release 时会自动释放CreateStreamOnHGlobal创建的 OLE 对象的内存。CreateStreamOnHGlobal函数签名如下：

HRESULT CreateStreamOnHGlobal( [in] HGLOBAL hGlobal, [in] BOOL fDeleteOnRelease, [out] LPSTREAM *ppstm );

由于这个OLE对象只占用了GlobalAlloc分配的部分内存，所以就出现了上述现象。

因此只要将上述代码中两处调用CreateStreamOnHGlobal函数的地方改成 FALSE 就可以了，不要自动释放内存即可。

修改后代码如下：

//显示截图窗口 void CMainFrame::OnOpenDesktop(ClientContext* pContext) { //...省略无关代码... HGLOBAL hGlobal = GlobalAlloc(GMEM_MOVEABLE, pContext->m_DeCompressionBuffer.GetBufferLen() - 1); void* pData = GlobalLock(hGlobal); memcpy(pData, pContext->m_DeCompressionBuffer.GetBuffer(1), pContext->m_DeCompressionBuffer.GetBufferLen() - 1); GlobalUnlock(hGlobal); IStream* pStream = NULL; if (CreateStreamOnHGlobal(hGlobal, FALSE, &pStream) == S_OK) { CImage image; if (SUCCEEDED(image.Load(pStream))) { IStream* pOutStream = NULL; if (CreateStreamOnHGlobal(NULL, FALSE, &pOutStream) == S_OK) { image.Save(Ttime); } } pStream->Release(); } GlobalFree(hGlobal); //...省略无关代码... }

问题6

如图所示，当使用被控使用UDP模式连接主控时，如果发生断线，被控再也无法连接上主控，也就是说被控UDP断线重连不起作用，影响所有需要用到网络连接的模块（例如上线模块、登录模块、键盘记录、远程屏幕等等）。

这段逻辑是当使用UDP连接时，发起连接后会调用WaitForSingleObject这个API等待连接结果（以m_hEvent_run这个内核Event受信为标志），连接成功会调用SetEvent这个API设置m_hEvent_run为受信状态。如下图所示：

然后调用CUdpSocket::run_event_loop()进入等待状态。

问题就在这里，调用CUdpSocket::run_event_loop()时将m_hEvent_run这个对象关闭了，一并受影响的还有m_hEvent这个Event对象。这样当断线重连时，再次调用上图中的CUdpSocket::Connect函数，由于m_hEvent_run已经被关闭，WaitForSingleObject立即返回，状态为WAIT_FAILED，导致永远连不上。如下图所示：

解决方法：