PVS-Studio 분석기를 사용하여 rdesktop 및 xrdp 확인

이것은 RDP 프로토콜 작업을 위한 오픈 소스 프로그램 테스트에 관한 일련의 기사 중 두 번째 리뷰입니다. 여기서는 rdesktop 클라이언트와 xrdp 서버를 살펴보겠습니다.

오류를 식별하는 도구로 사용됨 PVS-스튜디오. Windows, Linux 및 macOS 플랫폼에서 사용할 수 있는 C, C++, C# 및 Java 언어용 정적 코드 분석기입니다.

이 기사에는 나에게 흥미로웠던 오류만 제시되어 있습니다. 다만, 프로젝트 규모가 작아서 실수가 거의 없었습니다 :).

주의. FreeRDP 프로젝트 검증에 관한 이전 기사를 찾을 수 있습니다. 여기에.

데스크탑

데스크탑 — UNIX 기반 시스템용 RDP 클라이언트의 무료 구현입니다. Cygwin에서 프로젝트를 빌드하는 경우 Windows에서도 사용할 수 있습니다. GPLv3에 따라 라이센스가 부여되었습니다.

이 클라이언트는 매우 인기가 높습니다. 기본적으로 ReactOS에서 사용되며 이에 대한 타사 그래픽 프런트 엔드도 찾을 수 있습니다. 그러나 그는 꽤 나이가 많습니다. 그의 첫 번째 릴리스는 4년 2001월 17일에 이루어졌습니다. 이 글을 쓰는 당시 그는 XNUMX세였습니다.

앞서 언급했듯이 프로젝트는 매우 작습니다. 약 30줄의 코드가 포함되어 있는데, 오랜 세월을 고려하면 조금 이상합니다. 비교를 위해 FreeRDP에는 320만 줄이 포함되어 있습니다. Cloc 프로그램의 출력은 다음과 같습니다.

접근할 수 없는 코드

V779 사용할 수 없는 코드가 감지되었습니다. 오류가 있을 가능성이 있습니다. rdesktop.c 1502

int
main(int argc, char *argv[])
{
  ....
  return handle_disconnect_reason(deactivated, ext_disc_reason);

  if (g_redirect_username)
    xfree(g_redirect_username);

  xfree(g_username);
}

오류는 함수에서 즉시 발생합니다. 본관: 연산자 뒤에 오는 코드를 볼 수 있습니다. return — 이 조각은 메모리 정리를 수행합니다. 그러나 오류는 위협을 일으키지 않습니다. 할당된 모든 메모리는 프로그램이 종료된 후 운영 체제에 의해 지워집니다.

오류 처리 없음

V557 어레이 언더런이 가능합니다. 'n' 인덱스의 값은 -1에 도달할 수 있습니다. rdesktop.c 1872

RD_BOOL
subprocess(char *const argv[], str_handle_lines_t linehandler, void *data)
{
  int n = 1;
  char output[256];
  ....
  while (n > 0)
  {
    n = read(fd[0], output, 255);
    output[n] = ' '; // <=
    str_handle_lines(output, &rest, linehandler, data);
  }
  ....
}

이 경우 코드 조각은 파일이 끝날 때까지 파일에서 버퍼로 읽어옵니다. 그러나 여기에는 오류 처리가 없습니다. 문제가 발생하면 읽기 -1을 반환하고 배열이 오버런됩니다. 출력.

char 유형에서 EOF 사용

V739 EOF는 'char' 유형의 값과 비교되어서는 안 됩니다. '(c = fgetc(fp))'는 'int' 유형이어야 합니다. Ctrl.C 500

int
ctrl_send_command(const char *cmd, const char *arg)
{
  char result[CTRL_RESULT_SIZE], c, *escaped;
  ....
  while ((c = fgetc(fp)) != EOF && index < CTRL_RESULT_SIZE && c != 'n')
  {
    result[index] = c;
    index++;
  }
  ....
}

여기서는 파일 끝에 도달하는 잘못된 처리를 볼 수 있습니다. fgetc 코드가 0xFF인 문자를 반환하면 파일의 끝으로 해석됩니다(EOF).

EOF 이는 상수이며 일반적으로 -1로 정의됩니다. 예를 들어, CP1251 인코딩에서 러시아 알파벳의 마지막 문자는 코드 0xFF를 가지며, 다음과 같은 변수에 대해 이야기할 경우 숫자 -1에 해당합니다. 이륜 전차. 0xFF 기호는 다음과 같습니다. EOF (-1)은 파일의 끝으로 해석됩니다. 이러한 오류를 방지하기 위해 함수의 결과는 다음과 같습니다. fgetc 다음과 같은 변수에 저장해야 합니다. INT.

오타

조각 1

V547 'write_time' 표현식은 항상 false입니다. disk.c 805

RD_NTSTATUS
disk_set_information(....)
{
  time_t write_time, change_time, access_time, mod_time;
  ....
  if (write_time || change_time)
    mod_time = MIN(write_time, change_time);
  else
    mod_time = write_time ? write_time : change_time; // <=
  ....
}

아마도 이 코드의 작성자가 잘못했을 수도 있습니다. || и && 상태. 값에 대해 가능한 옵션을 고려해 보겠습니다. 쓰기_시간 и 변경_시간:

• 두 변수 모두 0과 같습니다. 이 경우 분기로 종료됩니다. 그렇지 않으면: 변수 mod_time 후속 조건에 관계없이 항상 0이 됩니다.
• 변수 중 하나가 0입니다. mod_time (다른 변수에 음수가 아닌 값이 있는 경우) 0과 같습니다. 왜냐하면 MIN 두 옵션 중 더 작은 옵션을 선택합니다.
• 두 변수 모두 0이 아닙니다. 최소값을 선택하세요.

조건을 다음과 같이 교체할 때 write_time && 변경_시간 동작이 올바르게 보일 것입니다:

• 하나 또는 두 변수 모두 0이 아닙니다. XNUMX이 아닌 값을 선택합니다.
• 두 변수 모두 0이 아닙니다. 최소값을 선택하세요.

조각 2

V547 표현은 항상 참입니다. 아마도 여기서는 '&&' 연산자를 사용해야 할 것입니다. disk.c 1419

static RD_NTSTATUS
disk_device_control(RD_NTHANDLE handle, uint32 request, STREAM in,
      STREAM out)
{
  ....
  if (((request >> 16) != 20) || ((request >> 16) != 9))
    return RD_STATUS_INVALID_PARAMETER;
  ....
}

여기 운영자도 헷갈리는거 같던데 || и &&또는 == и !=: 변수는 20과 9의 값을 동시에 가질 수 없습니다.

무제한 라인 복사

V512 'sprintf' 함수를 호출하면 'fullpath' 버퍼가 오버플로됩니다. disk.c 1257

RD_NTSTATUS
disk_query_directory(....)
{
  ....
  char *dirname, fullpath[PATH_MAX];
  ....
  /* Get information for directory entry */
  sprintf(fullpath, "%s/%s", dirname, pdirent->d_name);
  ....
}

함수 전체를 살펴보면 이 코드가 문제를 일으키지 않는다는 것이 분명해집니다. 그러나 미래에는 이러한 문제가 발생할 수 있습니다. 부주의하게 변경하면 버퍼 오버플로가 발생합니다. 스프린트 어떤 것에도 제한이 없으므로 경로를 연결할 때 배열의 경계를 넘어갈 수 있습니다. 이 호출을 확인하는 것이 좋습니다. snprintf(전체 경로, PATH_MAX, ....).

중복 조건

V560 조건식의 일부는 항상 참입니다: 추가 > 0. Scard.c 507

static void
inRepos(STREAM in, unsigned int read)
{
  SERVER_DWORD add = 4 - read % 4;
  if (add < 4 && add > 0)
  {
    ....
  }
}

Проверка 추가 > 0 여기서는 필요하지 않습니다. 변수는 항상 XNUMX보다 클 것입니다. % 4 읽음 나눗셈의 나머지 부분을 반환하지만 결코 4와 같을 수는 없습니다.

xrdp

xrdp — 오픈 소스 코드를 사용한 RDP 서버 구현. 이 프로젝트는 두 부분으로 나누어집니다:

• xrdp - 프로토콜 구현. Apache 2.0 라이센스에 따라 배포됩니다.
• xorgxrdp - xrdp와 함께 사용하기 위한 Xorg 드라이버 세트입니다. 라이센스 - X11(MIT와 유사하지만 광고에 사용이 금지됨)

프로젝트 개발은 rdesktop 및 FreeRDP의 결과를 기반으로 합니다. 처음에는 그래픽 작업을 위해 별도의 VNC 서버나 RDP를 지원하는 특수 X11 서버(X11rdp)를 사용해야 했지만 xorgxrdp의 출현으로 그 필요성이 사라졌습니다.

이 기사에서는 xorgxrdp를 다루지 않습니다.

xrdp 프로젝트는 이전 프로젝트와 마찬가지로 매우 작으며 약 80만 줄을 포함합니다.

더 많은 오타

V525 코드에는 유사한 블록 모음이 포함되어 있습니다. 87, 88, 89행의 'r', 'g', 'r' 항목을 확인하세요. rfxencode_rgb_to_yuv.c 87

static int
rfx_encode_format_rgb(const char *rgb_data, int width, int height,
                      int stride_bytes, int pixel_format,
                      uint8 *r_buf, uint8 *g_buf, uint8 *b_buf)
{
  ....
  switch (pixel_format)
  {
    case RFX_FORMAT_BGRA:
      ....
      while (x < 64)
      {
          *lr_buf++ = r;
          *lg_buf++ = g;
          *lb_buf++ = r; // <=
          x++;
      }
      ....
  }
  ....
}

이 코드는 RemoteFX용 jpeg2000 코덱을 구현하는 librfxcodec 라이브러리에서 가져온 것입니다. 여기서는 분명히 그래픽 데이터 채널이 혼합되어 있습니다. "파란색"대신 "빨간색"이 기록됩니다. 이 오류는 복사-붙여넣기 결과로 나타날 가능성이 높습니다.

비슷한 기능에서 같은 문제가 발생했습니다 rfx_encode_format_argb, 분석기는 또한 우리에게 다음과 같이 말했습니다.

V525 코드에는 유사한 블록 모음이 포함되어 있습니다. 260, 261, 262, 263행의 'a', 'r', 'g', 'r' 항목을 확인하세요. rfxencode_rgb_to_yuv.c 260

while (x < 64)
{
    *la_buf++ = a;
    *lr_buf++ = r;
    *lg_buf++ = g;
    *lb_buf++ = r;
    x++;
}

배열 선언

V557 배열 오버런이 가능합니다. 'i — 8' 인덱스의 값은 129에 도달할 수 있습니다. genkeymap.c 142

// evdev-map.c
int xfree86_to_evdev[137-8+1] = {
  ....
};

// genkeymap.c
extern int xfree86_to_evdev[137-8];

int main(int argc, char **argv)
{
  ....
  for (i = 8; i <= 137; i++) /* Keycodes */
  {
    if (is_evdev)
        e.keycode = xfree86_to_evdev[i-8];
    ....
  }
  ....
}

이 두 파일의 배열 선언과 정의는 호환되지 않습니다. 크기는 1씩 다릅니다. 그러나 오류는 발생하지 않습니다. 올바른 크기는 evdev-map.c 파일에 지정되어 있으므로 범위를 벗어나는 일이 없습니다. 따라서 이것은 쉽게 고칠 수 있는 버그일 뿐입니다.

잘못된 비교

V560 조건식의 일부는 항상 거짓입니다(cap_len < 0). xrdp_caps.c 616

// common/parse.h
#if defined(B_ENDIAN) || defined(NEED_ALIGN)
#define in_uint16_le(s, v) do 
....
#else
#define in_uint16_le(s, v) do 
{ 
    (v) = *((unsigned short*)((s)->p)); 
    (s)->p += 2; 
} while (0)
#endif

int
xrdp_caps_process_confirm_active(struct xrdp_rdp *self, struct stream *s)
{
  int cap_len;
  ....
  in_uint16_le(s, cap_len);
  ....
  if ((cap_len < 0) || (cap_len > 1024 * 1024))
  {
    ....
  }
  ....
}

함수는 유형 변수를 읽습니다. 서명되지 않은 짧은 다음과 같은 변수에 INT. 부호 없는 변수를 읽고 그 결과를 더 큰 변수에 할당하므로 여기서는 검사가 필요하지 않습니다. 따라서 변수는 음수 값을 가질 수 없습니다.

불필요한 점검

V560 조건식의 일부는 항상 참입니다(bpp != 16). libxrdp.c 704

int EXPORT_CC
libxrdp_send_pointer(struct xrdp_session *session, int cache_idx,
                     char *data, char *mask, int x, int y, int bpp)
{
  ....
  if ((bpp == 15) && (bpp != 16) && (bpp != 24) && (bpp != 32))
  {
      g_writeln("libxrdp_send_pointer: error");
      return 1;
  }
  ....
}

불평등 검사는 처음에 이미 비교가 있었기 때문에 여기서는 의미가 없습니다. 이는 오타일 가능성이 높으며 개발자가 연산자를 사용하려고 했습니다. || 잘못된 인수를 필터링합니다.

결론

감사 과정에서 심각한 오류는 발견되지 않았지만 많은 단점이 발견되었습니다. 그러나 이러한 디자인은 비록 범위는 작지만 많은 시스템에서 사용됩니다. 소규모 프로젝트라고 반드시 오류가 많은 것은 아니므로 소규모 프로젝트에서만 분석기의 성능을 판단해서는 안 됩니다. 이에 대한 자세한 내용은 "숫자로 확인된 감정".

당사에서 PVS-Studio 평가판을 다운로드할 수 있습니다. 온라인으로.

이 기사를 영어권 청중과 공유하려면 Sergey Larin 번역 링크를 사용하십시오. PVS-Studio로 rdesktop 및 xrdp 확인

출처 : habr.com