#377 rdesktop dumps core

[Martin Wheatley]

We're investigating two core dumps; both in ui_create_cursor()...

000209c4 ui_create_cursor (11, fffffffc, 0, 7c, 266315, 265b15) + 1dc
00032908 ???????? (a0324, 10, 0, 265b03, 265b15, 266315)
0003cb50 rdp5_process (a0324, 0, ffffffff, fffffff8, a0324, 0) + 19c
00031f80 ???????? (0, 3eb, ffff80, 0, 265aa1, 22ec00)
00033124 rdp_loop (ffbfebc0, ffbfebbc, 5b800, 13, 6, 58) + 18
000347a0 rdp_main_loop (ffbfebc0, ffbfebbc, ffbfee50, ffbfee10, 40, 1) + c
00019dac main (0, 0, 5c000, ffbfec10, ffbfef90, ffbfee10) + 584
000185a0 _start (0, 0, 0, 0, 0, 0) + 5c

In process_colour_pointer_common() in rdp.c I think that the sanitizing
of 'x' and 'y'...

    x = MAX(x, 0);
    x = MIN(x, width - 1);
    y = MAX(y, 0);
    y = MIN(y, height - 1);

should be...

    x = MIN(x, width - 1);
    x = MAX(x, 0);
    y = MIN(y, height - 1);
    y = MAX(y, 0);

to protect against 'width' and 'height' being zero - which currently
results in 'x' and/or 'y' going -ve.

Should there also be some protection against invalid/illegal
values for 'width' and 'height' before they are used...?

    width = MAX( width , 1 );
    height = MAX( height , 1 );

NB: Including the 'width' and 'height' protection means the existing 'x' and 'y'
protection is okay.

Whether this resolves our core dumps we will never know - but we are testing the suggested change

regards
Martin

[Henrik Andersson]

I need some more information on how to reproduce this crash, windows version, color depth X and remote, version of rdesktop etc...

[Henrik Andersson]

Might this report be related ?

https://sourceforge.net/p/rdesktop/feature-requests/161

[Martin Wheatley]

thanks for investigating this issue.

The problem was reported by one of my users two days running but he couldn't think of a correlation between the two crashes.

Feature Request 161 doesn't look the same as ours. In both cases the 'width' input parameter to ui_create_cursor() is zero which isn't the case in the other crash. Since we applied our suggested change in the sanitising we haven't had any reported crashes or other problems.

Not conclusive proof I admit.

The user who experienced these crashes is not available at the moment but I will ask if they can add any other information

regards

[Henrik Andersson]

Closing this issue due to no response. However I have fixed a few issue with the reading and clamping of cursor hotspot point in commit 1799.