Αυτή είναι η δεύτερη ανασκόπηση σε μια σειρά άρθρων σχετικά με τη δοκιμή προγραμμάτων ανοιχτού κώδικα για εργασία με το πρωτόκολλο RDP. Σε αυτό θα δούμε τον πελάτη rdesktop και τον διακομιστή xrdp.
Χρησιμοποιείται ως εργαλείο για τον εντοπισμό σφαλμάτων . Είναι ένας αναλυτής στατικού κώδικα για γλώσσες C, C++, C# και Java, διαθέσιμος σε πλατφόρμες Windows, Linux και macOS.
Το άρθρο παρουσιάζει μόνο εκείνα τα σφάλματα που μου φάνηκαν ενδιαφέροντα. Ωστόσο, τα έργα είναι μικρά, οπότε υπήρξαν λίγα λάθη :).
Σημείωση. Μπορείτε να βρείτε ένα προηγούμενο άρθρο σχετικά με την επαλήθευση έργου FreeRDP .
rdesktop
— δωρεάν υλοποίηση ενός προγράμματος-πελάτη RDP για συστήματα που βασίζονται σε UNIX. Μπορεί επίσης να χρησιμοποιηθεί στα Windows εάν δημιουργήσετε το έργο κάτω από το Cygwin. Άδεια χρήσης σύμφωνα με το GPLv3.
Αυτός ο πελάτης είναι πολύ δημοφιλής - χρησιμοποιείται από προεπιλογή στο ReactOS και μπορείτε επίσης να βρείτε γραφικά τρίτων για αυτόν. Ωστόσο, είναι αρκετά μεγάλος: η πρώτη του απελευθέρωση έγινε στις 4 Απριλίου 2001 - τη στιγμή που γράφει το άρθρο, είναι 17 ετών.
Όπως προανέφερα, το έργο είναι αρκετά μικρό. Περιέχει περίπου 30 χιλιάδες γραμμές κώδικα, κάτι που είναι λίγο περίεργο λαμβάνοντας υπόψη την ηλικία του. Για σύγκριση, το FreeRDP περιέχει 320 χιλιάδες γραμμές. Ακολουθεί η έξοδος του προγράμματος Cloc:
Απρόσιτος κωδικός
Εντοπίστηκε μη διαθέσιμος κωδικός. Είναι πιθανό να υπάρχει κάποιο σφάλμα. 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);
}Το σφάλμα μας συναντά αμέσως στη συνάρτηση κύριος: βλέπουμε τον κωδικό να ακολουθεί τον χειριστή απόδοση — αυτό το κομμάτι εκτελεί καθαρισμό μνήμης. Ωστόσο, το σφάλμα δεν αποτελεί απειλή: όλη η εκχωρημένη μνήμη θα διαγραφεί από το λειτουργικό σύστημα μετά την έξοδο του προγράμματος.
Κανένας χειρισμός σφαλμάτων
Είναι δυνατή η υποχώρηση συστοιχίας. Η τιμή του δείκτη '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);
}
....
}Το απόσπασμα κώδικα σε αυτήν την περίπτωση διαβάζεται από το αρχείο σε ένα buffer μέχρι να τελειώσει το αρχείο. Ωστόσο, δεν υπάρχει χειρισμός σφαλμάτων εδώ: αν κάτι πάει στραβά, τότε ανάγνωση θα επιστρέψει -1 και, στη συνέχεια, ο πίνακας θα υπερκεραστεί παραγωγή.
Χρήση ΕΟΦ σε τύπο char
Ο ΕΟΦ δεν πρέπει να συγκρίνεται με τιμή τύπου '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++;
}
....
}Εδώ βλέπουμε λανθασμένο χειρισμό της επίτευξης στο τέλος του αρχείου: if fgetc επιστρέφει έναν χαρακτήρα του οποίου ο κωδικός είναι 0xFF, θα ερμηνευτεί ως το τέλος του αρχείου (EOF).
EOF είναι μια σταθερά, που συνήθως ορίζεται ως -1. Για παράδειγμα, στην κωδικοποίηση CP1251, το τελευταίο γράμμα του ρωσικού αλφαβήτου έχει τον κωδικό 0xFF, ο οποίος αντιστοιχεί στον αριθμό -1 αν μιλάμε για μια μεταβλητή όπως δεξαμενή. Αποδεικνύεται ότι το σύμβολο 0xFF, όπως EOF Το (-1) ερμηνεύεται ως το τέλος του αρχείου. Για την αποφυγή τέτοιων σφαλμάτων, το αποτέλεσμα της συνάρτησης είναι fgetc πρέπει να αποθηκευτεί σε μια μεταβλητή όπως int.
Τυπογραφικά λάθη
Θραύσμα 1
Η έκφραση 'write_time' είναι πάντα ψευδής. δίσκος.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: επιλέξτε την ελάχιστη τιμή.
Κατά την αντικατάσταση της συνθήκης με χρόνος_γραφής & χρόνος_αλλαγής η συμπεριφορά θα φαίνεται σωστή:
- Η μία ή και οι δύο μεταβλητές δεν είναι ίση με 0: επιλέξτε μια μη μηδενική τιμή.
- Και οι δύο μεταβλητές δεν είναι ίσες με 0: επιλέξτε την ελάχιστη τιμή.
Θραύσμα 2
Η έκφραση είναι πάντα αληθινή. Μάλλον ο τελεστής '&&' θα πρέπει να χρησιμοποιείται εδώ. δίσκος.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 ταυτόχρονα.
Απεριόριστη αντιγραφή γραμμών
Μια κλήση της συνάρτησης 'sprintf' θα οδηγήσει σε υπερχείλιση του buffer 'fullpath'. δίσκος.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);
....
}Όταν κοιτάξετε πλήρως τη λειτουργία, θα καταστεί σαφές ότι αυτός ο κωδικός δεν προκαλεί προβλήματα. Ωστόσο, μπορεί να προκύψουν στο μέλλον: μια απρόσεκτη αλλαγή και θα έχουμε υπερχείλιση buffer - σπριντφ δεν περιορίζεται από τίποτα, οπότε όταν συνενώνουμε μονοπάτια μπορούμε να πάμε πέρα από τα όρια του πίνακα. Συνιστάται να παρατηρήσετε αυτήν την κλήση snprintf(fullpath, PATH_MAX,….).
Περιττή κατάσταση
Ένα μέρος της έκφρασης υπό όρους είναι πάντα αληθές: add > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Проверка προσθήκη > 0 δεν χρειάζεται εδώ: η μεταβλητή θα είναι πάντα μεγαλύτερη από το μηδέν, γιατί ανάγνωση % 4 θα επιστρέψει το υπόλοιπο της διαίρεσης, αλλά ποτέ δεν θα είναι ίσο με 4.
xrdp
— υλοποίηση διακομιστή RDP με ανοιχτό κώδικα. Το έργο χωρίζεται σε 2 μέρη:
- xrdp - υλοποίηση πρωτοκόλλου. Διανέμεται με την άδεια Apache 2.0.
- xorgxrdp - Ένα σύνολο προγραμμάτων οδήγησης Xorg για χρήση με xrdp. Άδεια χρήσης - X11 (όπως το MIT, αλλά απαγορεύει τη χρήση στη διαφήμιση)
Η ανάπτυξη του έργου βασίζεται στα αποτελέσματα του rdesktop και του FreeRDP. Αρχικά, για να εργαστείτε με γραφικά, έπρεπε να χρησιμοποιήσετε έναν ξεχωριστό διακομιστή VNC ή έναν ειδικό διακομιστή X11 με υποστήριξη RDP - X11rdp, αλλά με την εμφάνιση του xorgxrdp, η ανάγκη για αυτά εξαφανίστηκε.
Σε αυτό το άρθρο δεν θα καλύψουμε το xorgxrdp.
Το έργο xrdp, όπως και το προηγούμενο, είναι πολύ μικρό και περιέχει περίπου 80 χιλιάδες γραμμές.
Περισσότερα τυπογραφικά λάθη
Ο κώδικας περιέχει τη συλλογή παρόμοιων μπλοκ. Ελέγξτε τα στοιχεία 'r', 'g', 'r' στις γραμμές 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; // <=
x++;
}
....
}
....
}Αυτός ο κώδικας ελήφθη από τη βιβλιοθήκη librfxcodec, η οποία υλοποιεί τον κωδικοποιητή jpeg2000 για το RemoteFX. Εδώ, προφανώς, τα κανάλια δεδομένων γραφικών ανακατεύονται - αντί για το "μπλε" χρώμα, καταγράφεται το "κόκκινο". Αυτό το σφάλμα πιθανότατα εμφανίστηκε ως αποτέλεσμα αντιγραφής-επικόλλησης.
Το ίδιο πρόβλημα παρουσιάστηκε σε παρόμοια λειτουργία rfx_encode_format_argb, το οποίο μας είπε επίσης ο αναλυτής:
Ο κώδικας περιέχει τη συλλογή παρόμοιων μπλοκ. Ελέγξτε τα στοιχεία 'a', 'r', 'g', 'r' στις γραμμές 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++;
}Δήλωση πίνακα
Είναι δυνατή η υπέρβαση συστοιχίας. Η τιμή του δείκτη '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, επομένως δεν υπάρχουν εκτός ορίων. Επομένως, αυτό είναι απλώς ένα σφάλμα που μπορεί εύκολα να διορθωθεί.
Λανθασμένη σύγκριση
Ένα μέρος της έκφρασης υπό όρους είναι πάντα ψευδές: (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. Ο έλεγχος δεν χρειάζεται εδώ επειδή διαβάζουμε μια ανυπόγραφη μεταβλητή και εκχωρούμε το αποτέλεσμα σε μια μεγαλύτερη μεταβλητή, επομένως η μεταβλητή δεν μπορεί να λάβει αρνητική τιμή.
Περιττοί έλεγχοι
Ένα μέρος της έκφρασης υπό όρους είναι πάντα αληθές: (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.
Πηγή: www.habr.com