Hubinta FreeRDP iyadoo la adeegsanayo falanqeeyaha PVS-Studio

FreeRDP waa hirgelin il furan oo ka mid ah Borotokoolka Fog ee Desktop-ka (RDP), oo ah borotokool ay samaysay Microsoft oo loogu talagalay xakamaynta kombiyuutarka fog. Mashruucu wuxuu taageeraa goobo badan, oo ay ku jiraan Windows, Linux, macOS iyo xitaa iOS oo leh AndroidMashruucan waxaa loo doortay inuu noqdo kii ugu horreeyay ee taxane maqaallo ah oo loogu talagalay tijaabinta macaamiisha RDP iyadoo la adeegsanayo falanqaynta joogtada ah ee PVS-Studio.

Tiro yar oo taariikh ah

Mashruuca FreeRDP ayaa yimid ka dib markii Microsoft ay furtay qeexitaannada borotokoolka RDP ee lahaanshaha. Waqtigaas, waxaa jiray macmiil rdesktop ah, kaas oo hirgelintiisu ay ku saleysan tahay natiijooyinka injineernimada Dib-u-dejinta.

Markii hab-maamuuska la hirgeliyay, waxa aad u adkeyd in lagu daro hawlqabad cusub taas oo ay ugu wacan tahay qaab-dhismeedka mashruuca ee markaas jiray. Isbeddellada ku jira waxay keeneen khilaaf u dhexeeya horumariyayaal, taas oo keentay abuurista fargeeto ah rdsktop - FreeRDP. Qaybinta dheeraadka ah ee badeecada waxaa xaddiday shatiga GPLv2, taas oo ka dhalatay go'aan lagu gaaray in dib loogu celiyo Shatiga Apache v2. Si kastaba ha ahaatee, qof kastaa ma ogola inuu beddelo shatiga koodka, sidaas darteed horumariyayaashu waxay go'aansadeen inay dib u qoraan mashruuca, taasoo keentay codebase casri ah.

Waxaad wax badan oo ku saabsan taariikhda mashruuca ka akhriyi kartaa bogga rasmiga ah ee blog: "Taariikhda mashruuca FreeRDP".

Loo isticmaalo sidii qalab lagu aqoonsado khaladaadka iyo dayacanka ka iman kara koodhka. PVS-StudioWaa falanqeeye koodhka oo taagan oo loogu talagalay C, C++, C# iyo Java, oo laga heli karo goobaha kala duwan Windows, Linux и macOS.

Maqaalku wuxuu soo bandhigayaa oo kaliya khaladaadkaas oo ii muuqday kuwo aad ii xiisa badan.

Xusuusta oo daadato

V773 Hawsha waa laga baxay iyada oo aan la sii dayn tilmaamayaasha 'cwd'. Diiditaanka xusuusta ayaa suurtagal ah. deegaanka.c 84

DWORD GetCurrentDirectoryA(DWORD nBufferLength, LPSTR lpBuffer)
{
  char* cwd;
  ....
  cwd = getcwd(NULL, 0);
  ....
  if (lpBuffer == NULL)
  {
    free(cwd);
    return 0;
  }

  if ((length + 1) > nBufferLength)
  {
    free(cwd);
    return (DWORD) (length + 1);
  }

  memcpy(lpBuffer, cwd, length + 1);
  return length;
  ....
}

Qaybtan waxaa laga soo qaatay nidaamka hoose ee winpr, kaas oo hirgeliya duubista WINAPI ee aan ahaynWindows nidaamyada, tusaale ahaan, waa nooc khafiif ah oo Wine ah. Halkan waxaad ku arki kartaa daadasho: xusuusta oo loo qoondeeyay shaqada. helicwd, waxa la sii daayaa kaliya marka la xalinayo kiisaska gaarka ah. Si loo saxo qaladka waxaad u baahan tahay inaad wacdo ku darto free после xaashi.

Diyaar garow ka baxsan xadka

V557 Array xad dhaaf ah waa suurtagal. Qiimaha 'event->EventHandlerCount' index wuxuu gaari karaa 32. PubSub.c 117

#define MAX_EVENT_HANDLERS  32

struct _wEventType
{
  ....
  int EventHandlerCount;
  pEventHandler EventHandlers[MAX_EVENT_HANDLERS];
};

int PubSub_Subscribe(wPubSub* pubSub, const char* EventName,
      pEventHandler EventHandler)
{
  ....
  if (event->EventHandlerCount <= MAX_EVENT_HANDLERS)
  {
    event->EventHandlers[event->EventHandlerCount] = EventHandler;
    event->EventHandlerCount++;
  }
  ....
}

Tusaalahani wuxuu liiska ku darayaa shay cusub xataa haddii tirada curiyayaasha ay gaadheen ugu badnaan. Halkan waa ku filan in la beddelo hawlwadeenka <= on <, si aan loo dhaafin xudduudaha safafka.

Khalad kale oo noocan ah ayaa la helay:

• V557 Array xad dhaaf ah waa suurtagal. Qiimaha 'iBitmapFormat' index wuxuu gaari karaa 8. orders.c 2623

Typos

Jajab 1

V547 Odhaahda '! pipe->In' had iyo jeer waa been. MessagePipe.c 63

wMessagePipe* MessagePipe_New()
{
  ....
  pipe->In = MessageQueue_New(NULL);
  if (!pipe->In)
    goto error_in;

  pipe->Out = MessageQueue_New(NULL);
  if (!pipe->In) // <=
    goto error_out;
  ....
}

Halkan waxaan ku aragnaa typo caadi ah: xaaladda labaad waxay hubisaa isla doorsoomiyaha koowaad. Inta badan, khaladku wuxuu u muuqday natiijada koobiyeynta kood lagu guul darraystay.

Jajab 2

V760 Laba baloog oo qoraal isku mid ah ayaa la helay. Qeybta labaad waxay ka bilaabataa xariiqda 771. tsg.c 770

typedef struct _TSG_PACKET_VERSIONCAPS
{
  ....
  UINT16 majorVersion;
  UINT16 minorVersion;
  ....
} TSG_PACKET_VERSIONCAPS, *PTSG_PACKET_VERSIONCAPS;

static BOOL TsProxyCreateTunnelReadResponse(....)
{
  ....
  PTSG_PACKET_VERSIONCAPS versionCaps = NULL;
  ....
  /* MajorVersion (2 bytes) */
  Stream_Read_UINT16(pdu->s, versionCaps->majorVersion);
  /* MinorVersion (2 bytes) */
  Stream_Read_UINT16(pdu->s, versionCaps->majorVersion);
  ....
}

Qoraal kale: faallooyinka code ayaa tilmaamaya in duntu ay tahay inay timaado smallVersion, si kastaba ha ahaatee, akhrisku wuxuu ku dhacaa doorsoome la magacaabay nooca weyn. Si kastaba ha ahaatee, anigu aqoon uma lihi borotokoolka, markaa tani waa uun male.

Jajab 3

V524 Waa wax lala yaabo in shaqada 'trio_index_last' ay si buuxda ula mid tahay jirka 'trio_index' function. triostr.c 933

/**
   Find first occurrence of a character in a string.
   ....
 */
TRIO_PUBLIC_STRING char *
trio_index
TRIO_ARGS2((string, character),
     TRIO_CONST char *string,
     int character)
{
  assert(string);
  return strchr(string, character);
}

/**
   Find last occurrence of a character in a string.
   ....
 */
TRIO_PUBLIC_STRING char *
trio_index_last
TRIO_ARGS2((string, character),
     TRIO_CONST char *string,
     int character)
{
  assert(string);
  return strchr(string, character);
}

Marka la eego faallooyinka, shaqada saddex_index ka helo xarfaha ugu horreeya ee ku habboon xargaha marka trio_index_ugu dambeeyay - wax u dambeeya. Laakiin jidhka hawlahani waa isku mid! Waxay u badan tahay in tani ay tahay typo, iyo shaqada trio_index_ugu dambeeyay loo baahan yahay in la isticmaalo strhrchr halkii strchr. Markaa hab-dhaqanka ayaa la filayaa.

Jajab 4

V769 Tilmaamaha 'xogta' ee tibaaxaha wuxuu la mid yahay nullptr. Qiimaha ka dhalanaya ee hawlgallada xisaabeed ee tilmaamayaashan waa macno darro mana aha in la isticmaalo. nsc_encode.c 124

static BOOL nsc_encode_argb_to_aycocg(NSC_CONTEXT* context,
                                      const BYTE* data,
                                      UINT32 scanline)
{
  ....
  if (!context || data || (scanline == 0))
    return FALSE;
  ....
  src = data + (context->height - 1 - y) * scanline;
  ....
}

Waxay u egtahay in hawlwadeenka diidmada halkan si lama filaan ah loo waayay ! Dhow data. Waxaa la yaab leh in tani ay tahay mid aan la dareemin.

Jajab 5

V517 Isticmaalka 'if (A) {…} kale haddii (A) {…}' qaab la ogaaday. Waxaa jirta suurtagalnimada joogitaanka qalad macquul ah. Eeg khadadka: 213, 222. rdpei_common.c 213

BOOL rdpei_write_4byte_unsigned(wStream* s, UINT32 value)
{
  BYTE byte;

  if (value <= 0x3F)
  {
    ....
  }
  else if (value <= 0x3FFF)
  {
    ....
  }
  else if (value <= 0x3FFFFF)
  {
    byte = (value >> 16) & 0x3F;
    Stream_Write_UINT8(s, byte | 0x80);
    byte = (value >> 8) & 0xFF;
    Stream_Write_UINT8(s, byte);
    byte = (value & 0xFF);
    Stream_Write_UINT8(s, byte);
  }
  else if (value <= 0x3FFFFF)
  {
    byte = (value >> 24) & 0x3F;
    Stream_Write_UINT8(s, byte | 0xC0);
    byte = (value >> 16) & 0xFF;
    Stream_Write_UINT8(s, byte);
    byte = (value >> 8) & 0xFF;
    Stream_Write_UINT8(s, byte);
    byte = (value & 0xFF);
    Stream_Write_UINT8(s, byte);
  }
  ....
}

Labada shuruudood ee ugu dambeeya waa isku mid: sida muuqata qof ayaa ilaaway inuu hubiyo iyaga ka dib koobiyeynta. Waxaa laga dareemi karaa koodhka in qaybta ugu dambeysa ay la shaqeyso qiyamka afar-byte, markaa waxaan u qaadan karnaa in xaaladda ugu dambeysa ay tahay qiimaha <= 0x3FFFFFFFF.

Khalad kale oo noocan ah ayaa la helay:

• V517 Isticmaalka 'haddii (A) {…} kale haddii (A) {…}' qaabka la ogaaday. Waxaa jirta suurtagalnimada joogitaanka qalad macquul ah. Eeg sadarrada: 169, 173. file.c 169

Xaqiijinta xogta gelinta

Jajab 1

V547 Odhaahda 'strcat( bartilmaameed, isha) != NULL' had iyo jeer waa run. triostr.c 425

TRIO_PUBLIC_STRING int
trio_append
TRIO_ARGS2((target, source),
     char *target,
     TRIO_CONST char *source)
{
  assert(target);
  assert(source);
  
  return (strcat(target, source) != NULL);
}

Hubinta natiijada shaqada ee tusaalahan waa khalad. Shaqada cidhiidhi ku soo celiyaa tilmaame nooca ugu dambeeya ee xadhigga, i.e. halbeeggii ugu horeeyey baa maray. Xaaladdan oo kale waa bartilmaameed. Si kastaba ha ahaatee, haddii ay siman tahay waxba kama, ka dibna aad bay u daahday in la hubiyo, tan iyo shaqada cidhiidhi waa la tixraaci doonaa.

Jajab 2

V547 Odhaahda 'cache' had iyo jeer waa run. glyph.c 730

typedef struct rdp_glyph_cache rdpGlyphCache;

struct rdp_glyph_cache
{
  ....
  GLYPH_CACHE glyphCache[10];
  ....
};

void glyph_cache_free(rdpGlyphCache* glyphCache)
{
  ....
  GLYPH_CACHE* cache = glyphCache->glyphCache;

  if (cache)
  {
    ....
  }
  ....
}

Xaaladdan oo kale doorsoomayaasha khasnado cinwaanka array taagan ayaa loo qoondeeyay glyphCache-> glyphCache. Haddaba, hubi haddii (cache) waa laga tagi karaa.

Khaladka maareynta kheyraadka

V1005 Kheyraadka waxaa lagu helay iyadoo la adeegsanayo 'CreateFileA' function laakiin waxaa la sii daayay iyadoo la adeegsanayo 'fclose' aan ku habboonayn. shahaadada.c 447

BOOL certificate_data_replace(rdpCertificateStore* certificate_store,
                              rdpCertificateData* certificate_data)
{
  HANDLE fp;
  ....
  fp = CreateFileA(certificate_store->file, GENERIC_READ | GENERIC_WRITE, 0,
                   NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
  ....
  if (size < 1)
  {
    CloseHandle(fp);
    return FALSE;
  }
  ....
  if (!data)
  {
    fclose(fp);
    return FALSE;
  }
  ....
}

Sharaxa faylka fp, uu sameeyay wicitaan shaqo CreateFile xidhay si qalad ah by function fclose ka maktabadda caadiga ah, ma Gacanta xir.

Shuruudo isku mid ah

V581 Tibaaxaha shuruudaha ah ee 'haddii' weedhaha ku ag yaal waa isku mid. Eeg khadadka: 269, 283. ndr_structure.c 283

void NdrComplexStructBufferSize(PMIDL_STUB_MESSAGE pStubMsg,
      unsigned char* pMemory, PFORMAT_STRING pFormat)
{
  ....
  if (conformant_array_description)
  {
    ULONG size;
    unsigned char array_type;
    array_type = conformant_array_description[0];
    size = NdrComplexStructMemberSize(pStubMsg, pFormat);
    WLog_ERR(TAG, "warning: NdrComplexStructBufferSize array_type: "
      "0x%02X unimplemented", array_type);
    NdrpComputeConformance(pStubMsg, pMemory + size,
      conformant_array_description);
    NdrpComputeVariance(pStubMsg, pMemory + size,
      conformant_array_description);
    MaxCount = pStubMsg->MaxCount;
    ActualCount = pStubMsg->ActualCount;
    Offset = pStubMsg->Offset;
  }

  if (conformant_array_description)
  {
    unsigned char array_type;
    array_type = conformant_array_description[0];
    pStubMsg->MaxCount = MaxCount;
    pStubMsg->ActualCount = ActualCount;
    pStubMsg->Offset = Offset;
    WLog_ERR(TAG, "warning: NdrComplexStructBufferSize array_type: "
      "0x%02X unimplemented", array_type);
  }
  ....
}

Tusaalahani waxa laga yaabaa inaanu ahayn bug. Si kastaba ha ahaatee, labada xaaladoodba waxay ka kooban yihiin fariimo isku mid ah, kuwaas oo mid ka mid ah ay u badan tahay in meesha laga saaro.

Nadiifinta tilmaameyaal aan jirin

V575 Tilmaamaha aan waxba tarayn waxa loo gudbiyaa shaqada 'free'. Eeg dooda kowaad. smartcard_pcsc.c 875

WINSCARDAPI LONG WINAPI PCSC_SCardListReadersW(
  SCARDCONTEXT hContext,
  LPCWSTR mszGroups,
  LPWSTR mszReaders,
  LPDWORD pcchReaders)
{
  LPSTR mszGroupsA = NULL;
  ....
  mszGroups = NULL; /* mszGroups is not supported by pcsc-lite */

  if (mszGroups)
    ConvertFromUnicode(CP_UTF8,0, mszGroups, -1, 
                       (char**) &mszGroupsA, 0,
                       NULL, NULL);

  status = PCSC_SCardListReaders_Internal(hContext, mszGroupsA,
                                          (LPSTR) &mszReadersA,
                                          pcchReaders);

  if (status == SCARD_S_SUCCESS)
  {
    ....
  }

  free(mszGroupsA);
  ....
}

Si aad u shaqeyso free waxaad dhaafi kartaa tilmaame aan waxba ka jirin, falanqeeyuhuna wuu og yahay. Laakin haddii xaalad la ogaado in tilmaamuhu mar walba la marsiiyo waxba, sida ku qoran qaybtan, digniin ayaa la soo saari doonaa.

Tilmaame mszGroupsA marka hore waa siman yihiin waxba kama oo aan meel kale lagu bilaabin. Laanta koodka kaliya ee tilmaamuhu la bilaabi karo waa mid aan la gaadhi karin.

Waxaa jiray fariimo kale sida:

• V575 Tilmaamaha waxba kama jiraan waxa loo gudbiyaa shaqada 'free'. Eeg dooda kowaad. shatiga.c 790
• V575 Tilmaamaha waxba kama jiraan waxa loo gudbiyaa shaqada 'free'. Eeg dooda kowaad. rdpsnd_alsa.c 575

Inta badan, doorsoomayaal la ilaaway ayaa soo baxa inta lagu jiro habka dib-u-soo-nooleynta oo si fudud ayaa looga saari karaa.

Qulqulka suurtagalka ah

V1028 Qulqulka suurtagalka ah Tixgeli inaad tuurto operands, ma aha natiijada. makecert.c 1087

// openssl/x509.h
ASN1_TIME *X509_gmtime_adj(ASN1_TIME *s, long adj);

struct _MAKECERT_CONTEXT
{
  ....
  int duration_years;
  int duration_months;
};

typedef struct _MAKECERT_CONTEXT MAKECERT_CONTEXT;

int makecert_context_process(MAKECERT_CONTEXT* context, ....)
{
  ....
  if (context->duration_months)
    X509_gmtime_adj(after, (long)(60 * 60 * 24 * 31 *
      context->duration_months));
  else if (context->duration_years)
    X509_gmtime_adj(after, (long)(60 * 60 * 24 * 365 *
      context->duration_years));
  ....
}

Keenida natiijada dheer ma aha ilaalinta qulqulka, maadaama xisaabinta lafteeda ay dhacdo iyadoo la adeegsanayo nooca INT.

Tixraaca tilmaanta ee bilowga

V595 Tilmaamaha 'context' ayaa la isticmaalay ka hor inta aan laga xaqiijin nullptr. Eeg khadadka: 746, 748. gfx.c 746

static UINT gdi_SurfaceCommand(RdpgfxClientContext* context,
                               const RDPGFX_SURFACE_COMMAND* cmd)
{
  ....
  rdpGdi* gdi = (rdpGdi*) context->custom;

  if (!context || !cmd)
    return ERROR_INVALID_PARAMETER;
  ....
}

Waa kan tilmaanta Macnaha guud waa la diiday inta lagu guda jiro bilowga - ka hor inta aan la hubin.

Khaladaadka kale ee noocan ah ayaa la helay:

• V595 Tilmaamaha 'ntlm' waa la isticmaalay ka hor inta aan laga xaqiijin nullptr. Eeg khadadka: 236, 255. ntlm.c 236
• V595 Tilmaamaha 'context' ayaa la isticmaalay ka hor inta aan laga xaqiijin nullptr. Hubi khadadka: 1003, 1007. rfx.c 1003
• V595 Tilmaamaha 'rdpei' ayaa la isticmaalay ka hor inta aan laga xaqiijin nullptr. Eeg khadadka: 176, 180. rdpei_main.c 176
• V595 Tilmaamaha 'gdi' waa la isticmaalay ka hor inta aan laga xaqiijin nullptr. Eeg khadadka: 121, 123. xf_gfx.c 121

Xaalad aan macno lahayn

V547 Odhaahda 'rdp->state>= CONNECTION_STATE_ACTIVE' had iyo jeer waa run. isku xidhka.c 1489

int rdp_server_transition_to_state(rdpRdp* rdp, int state)
{
  ....
  switch (state)
  {
    ....
    case CONNECTION_STATE_ACTIVE:
      rdp->state = CONNECTION_STATE_ACTIVE;          // <=
      ....
      if (rdp->state >= CONNECTION_STATE_ACTIVE)     // <=
      {
        IFCALLRET(client->Activate, client->activated, client);

        if (!client->activated)
          return -1;
      }
    ....
  }
  ....
}

Way fududahay in la arko in xaaladda koowaad aysan macno lahayn sababtoo ah qiimaha u dhigma ee hore loo qoondeeyey.

Xadhig khaldan

V576 Qaab khaldan Tixgeli inaad hubiso dooda saddexaad ee dhabta ah ee shaqada 'sscanf'. Tilmaanta nooca int ee aan saxeexin ayaa la filayaa. wakiil.c 220

V560 Qayb ka mid ah tibaaxaha shuruudaysan had iyo jeer waa run: (rc>= 0). wakiil.c 222

static BOOL check_no_proxy(....)
{
  ....
  int sub;
  int rc = sscanf(range, "%u", &sub);

  if ((rc == 1) && (rc >= 0))
  {
    ....
  }
  ....
}

Falanqeeyaha ayaa isla markiiba soo saaray 2 digniin oo jajabkan. Tilmaame %u wuxuu filayaa doorsoome nooc ah saxiixan int, laakiin doorsooma sub nooc leh INT. Marka xigta waxaan aragnaa jeeg shaki leh: xaaladda dhinaca midig ma aha mid macno leh, tan iyo bilowgii waxaa jira isbarbardhigga mid. Ma garanayo waxa qoraaga koodkan ula jeedo, laakiin wax cad ayaa halkan ka khaldan.

Jeegaga ka baxsan dalabka

V547 Odhaahda 'xaaladda == 0x00090314' had iyo jeer waa been. ntlm.c 299

BOOL ntlm_authenticate(rdpNtlm* ntlm, BOOL* pbContinueNeeded)
{
  ....
  if (status != SEC_E_OK)
  {
    ....
    return FALSE;
  }

  if (status == SEC_I_COMPLETE_NEEDED)            // <=
    status = SEC_E_OK;
  else if (status == SEC_I_COMPLETE_AND_CONTINUE) // <=
    status = SEC_I_CONTINUE_NEEDED;
  ....
}

Shuruudaha la hubiyay waxay had iyo jeer noqon doonaan been, maadaama fulinta kaliya ay gaari doonto shuruudda labaad haddii heerka == SEC_E_OK. Koodhka saxda ah wuxuu u ekaan karaa sidan:

if (status == SEC_I_COMPLETE_NEEDED)
  status = SEC_E_OK;
else if (status == SEC_I_COMPLETE_AND_CONTINUE)
  status = SEC_I_CONTINUE_NEEDED;
else if (status != SEC_E_OK)
{
  ....
  return FALSE;
}

gunaanad

Sidaa darteed, hubinta mashruuca ayaa muujisay dhibaatooyin badan, laakiin kaliya qaybta ugu xiisaha badan ayaa lagu sharaxay maqaalka. Horumarinta mashruuca laftoodu waxay hubin karaan mashruuca iyagoo ka codsanaya furaha shatiga ku meel gaadhka ah ee shabakada PVS-Studio. Waxa kale oo jiray waxyaabo been abuur ah, kuwaas oo ka shaqayn doona hagaajinta falanqeeyayaasha. Si kastaba ha noqotee, falanqaynta joogtada ah waa muhiim haddii aad rabto inaadan wanaajin tayada code-kaaga, laakiin sidoo kale inaad yarayso wakhtiga lagu raadinayo khaladaadka, PVS-Studio ayaa kaa caawin kara tan.

Haddii aad rabto inaad maqaalkan la wadaagto dhagaystayaasha ku hadla Ingiriisiga, fadlan isticmaal xidhiidhka tarjumaadda: Sergey Larin. Ku hubinta FreeRDP PVS-Studio

Source: www.habr.com