Esta é a segunda revisão de uma série de artigos sobre a verificação de programas de código aberto para trabalhar com o protocolo RDP. Nele, consideraremos o cliente rdesktop e o servidor xrdp.
O PVS-Studio foi usado como uma ferramenta para detectar erros. É um analisador de código estático para C, C ++, C # e Java, disponível no Windows, Linux e macOS.
O artigo apresenta apenas os erros que me pareciam interessantes. No entanto, os projetos são pequenos, por isso houve poucos erros :).
Nota O artigo anterior sobre a verificação do projeto FreeRDP pode ser encontrado
aqui .
rdesktop
O rdesktop é uma implementação gratuita do cliente RDP para sistemas baseados em UNIX. Também pode ser usado no Windows, se você criar um projeto no Cygwin. Licenciado sob GPLv3.
Esse cliente é muito popular - ele é usado por padrão no ReactOS e você também pode encontrar front-ends gráficos de terceiros. No entanto, ele é bastante velho: o primeiro lançamento ocorreu em 4 de abril de 2001 - na época em que escrevia, tinha 17 anos.
Como observei anteriormente, o projeto é muito pequeno. Ele contém cerca de 30 mil linhas de código, o que é um pouco estranho, dada a sua idade. Para comparação, o FreeRDP contém 320 mil linhas. Aqui está a saída do programa Cloc:
Código inacessível
V779 Código inacessível detectado. É possível que haja um erro. 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); }
O erro nos encontra imediatamente na função
principal : vemos o código que vem após a
declaração de retorno - esse fragmento limpa a memória. No entanto, o erro não representa uma ameaça: toda a memória alocada é limpa pelo sistema operacional após o término do programa.
Falta de tratamento de erros
V557 Array underrun é possível. O valor do índice 'n' pode chegar a -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] = '\0';
O trecho de código nesse caso lê do arquivo para o buffer até que o arquivo termine. No entanto, não há manipulação de erros aqui: se algo der errado, a
leitura retornará -1 e a matriz de saída irá além dos limites da matriz.
Usando EOF em char
O V739 EOF não deve ser comparado com um valor do tipo 'char'. O '(c = fgetc (fp))' deve ser do tipo '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++; } .... }
Aqui vemos um processamento incorreto para atingir o final do arquivo: se
fgetc retornar um caractere cujo código é 0xFF, ele será interpretado como o final do arquivo (
EOF ).
EOF é uma constante, geralmente definida como -1. Por exemplo, na codificação CP1251, a última letra do alfabeto russo tem o código 0xFF, que corresponde ao número -1, se estivermos falando de uma variável do tipo
char . Acontece que o caractere 0xFF, como
EOF (-1), é percebido como o final do arquivo. Para evitar esses erros, o resultado da função
fgetc deve ser armazenado em uma variável
int .
Erros de digitação
Fragmento 1A expressão
V547 'write_time' é sempre falsa. 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;
Talvez o autor deste código tenha confundido
|| e
&& fornecidos. Considere as opções possíveis para
write_time e
change_time :
- Ambas as variáveis são 0: nesse caso, chegamos ao ramo else : a variável mod_time sempre será 0, independentemente da seguinte condição.
- Uma das variáveis é 0: mod_time será 0 (desde que a outra variável tenha um valor não negativo), porque o MIN escolherá a menor das duas opções.
- Ambas as variáveis não são iguais a 0: selecionamos o valor mínimo.
Ao substituir uma condição por
write_time && change_time, o comportamento parecerá correto:
- Uma ou ambas as variáveis não são iguais a 0: selecione um valor diferente de zero.
- Ambas as variáveis não são iguais a 0: selecionamos o valor mínimo.
Fragmento 2A expressão
V547 é sempre verdadeira. Provavelmente o operador '&&' deve ser usado aqui. 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; .... }
Aparentemente, os operadores
|| e
&& ,
== e
! = : a variável não pode aceitar os valores 20 e 9 ao mesmo tempo.
Cópia de linha ilimitada
V512 Uma chamada da função 'sprintf' levará ao estouro do buffer 'caminho completo'. disk.c 1257
RD_NTSTATUS disk_query_directory(....) { .... char *dirname, fullpath[PATH_MAX]; .... sprintf(fullpath, "%s/%s", dirname, pdirent->d_name); .... }
Ao considerar a função, ficará completamente claro que esse código não causa problemas. No entanto, eles podem surgir no futuro: uma mudança descuidada e obtemos um estouro de buffer - o
sprintf não é limitado por nada; portanto, ao concatenar caminhos, podemos ir além dos limites da matriz. É recomendável observar esta chamada no
snprintf (caminho completo, PATH_MAX, ....) .
Condição redundante
V560 Uma parte da expressão condicional é sempre verdadeira: add> 0. scard.c 507
static void inRepos(STREAM in, unsigned int read) { SERVER_DWORD add = 4 - read % 4; if (add < 4 && add > 0) { .... } }
Marcar
add> 0 é inútil: a variável sempre será maior que zero, porque a
leitura% 4 retornará o restante da divisão e nunca será 4.
xrdp
xrdp é uma implementação de servidor RDP de código aberto. O projeto está dividido em 2 partes:
- xrdp - implementação de protocolo. Distribuído sob a licença Apache 2.0.
- xorgxrdp - Um conjunto de drivers Xorg para uso com o xrdp. Licença - X11 (como MIT, mas proíbe o uso em publicidade)
O desenvolvimento do projeto é baseado nos resultados do rdesktop e do FreeRDP. Inicialmente, tive que usar um servidor VNC separado para trabalhar com gráficos, ou um servidor X11 especial com suporte a RDP - X11rdp, mas com o advento do xorgxrdp, eles não eram mais necessários.
Neste artigo, não tocaremos no xorgxrdp.
O projeto xrdp, como o anterior, é muito pequeno e contém cerca de 80 mil linhas.
Mais erros de digitação
V525 O código contém a coleção de blocos semelhantes. Verifique os itens 'r', 'g', 'r' nas linhas 87, 88, 89. 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;
Esse código foi retirado da biblioteca librfxcodec que implementa o codec jpeg2000 para que o RemoteFX funcione. Aqui, aparentemente, os canais de dados gráficos são misturados - em vez da cor "azul", "vermelho" é gravado. Esse erro provavelmente apareceu como resultado de copiar e colar.
O mesmo problema ocorreu em uma função semelhante
rfx_encode_format_argb , que o analisador também nos falou sobre:
V525 O código contém a coleção de blocos semelhantes. Verifique os itens 'a', 'r', 'g', 'r' nas linhas 260, 261, 262, 263. rfxencode_rgb_to_yuv.c 260
while (x < 64) { *la_buf++ = a; *lr_buf++ = r; *lg_buf++ = g; *lb_buf++ = r; x++; }
Declaração de matriz
A saturação da matriz V557 é possível. O valor do índice 'i - 8' pode chegar a 129. genkeymap.c 142
A declaração e a definição da matriz nesses dois arquivos são incompatíveis - o tamanho difere de 1. No entanto, nenhum erro ocorre - o tamanho correto é indicado no arquivo evdev-map.c, portanto, não há como escapar dos limites. Portanto, essa é apenas uma falha fácil de corrigir.
Comparação inválida
V560 Uma parte da expressão condicional é sempre falsa: (cap_len <0). xrdp_caps.c 616
Em uma função, uma variável do tipo
sem assinatura curta é lida em uma variável do tipo
int . A verificação não é necessária aqui, porque lemos uma variável de um tipo não assinado e atribuímos o resultado a uma variável maior, para que a variável não possa assumir um valor negativo.
Verificações desnecessárias
V560 Uma parte da expressão condicional é sempre verdadeira: (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; } .... }
Verificações de desigualdade não fazem sentido aqui, porque já temos uma comparação no início. É provável que este seja um erro de digitação e o desenvolvedor desejou usar o
|| para filtrar argumentos inválidos.
Conclusão
Durante a inspeção, nenhum erro sério foi detectado, mas muitas deficiências foram encontradas. No entanto, esses projetos são usados em muitos sistemas, embora de escopo pequeno. Um projeto pequeno não precisa ter muitos erros; portanto, você não deve julgar o trabalho do analisador apenas em projetos pequenos. Você pode ler mais sobre isso no artigo "
Sensações, confirmadas por números ".
Você pode baixar uma versão de avaliação do PVS-Studio em nosso
site .
Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Sergey Larin.
Verificando o rdesktop e o xrdp com o PVS-Studio