<html> <title>Selection Bug</title> <body> <table> <tr><td><!-- empty cell --></td></tr> <tr><td> Select beyond the end of the text and Dillo segfaults. </td></tr> <!-- missing </table> --> </body> </html> <!-- (gdb) bt 2 #0 Selection_correct_char_pos (it=0x0, char_pos=0) at selection.c:405 #1 0x080686a6 in Selection_adjust_selection (selection=0x80dcf70, it=0x80e5ce0, char_pos=0) at selection.c:335 (More stack frames follow...) (gdb) list selection.c:335 330 DwExtIterator *new_to; 331 gint new_to_char, cmp_old, cmp_new, cmp_diff, len; 332 gboolean brute_highlighting = FALSE; 333 334 new_to = a_Dw_ext_iterator_new_variant (selection->to, it); 335 new_to_char = Selection_correct_char_pos (new_to, char_pos); 336 337 cmp_old = a_Dw_ext_iterator_compare (selection->to, selection->from);338 cmp_new = a_Dw_ext_iterator_compare (new_to, selection->from); 339 I see 3 possible attacks on this one: 1. Add a check for NULL on the return value of a_Dw_ext_iterator_new_variant(). 2. Do that special empty DwExtIterator idea you mentioned. 3. Make html.c clean up the <table> tag better. The missing closer probably shouldn't make a difference in this example, but the segfault only happens if the closer is missing. Cheers, Joe Crayne -->
Hi, FYI: I seem to have fixed the bug, expect it to be committed today. Those interested should look at the differences in the code, it is only a simple coding bug, nothing very interesting. On Fri, Jul 11, Joe Crayne wrote:
<html> <title>Selection Bug</title> <body>
<table> <tr><td><!-- empty cell --></td></tr>
<tr><td> Select beyond the end of the text and Dillo segfaults. </td></tr> <!-- missing </table> -->
</body> </html>
The reason, why the missing </table> tag matters, is that Html_close_tag_table() will insert a break after the table, which makes iterators behave different.
[...] I see 3 possible attacks on this one:
1. Add a check for NULL on the return value of a_Dw_ext_iterator_new_variant().
a_Dw_ext_iterator_new() returns NULL, if the page does not contain any non-widget contents. After it one succeeded, it should always succeed. That it does not, was a bug, and is fixed now. BTW: Actually, this is only true, as long as no contents is *removed* from the page, which does not yet happen. If it happens, the selection will anyway get more complicated, a check in this case would not make much sense.
2. Do that special empty DwExtIterator idea you mentioned.
Yes, this will probably part of 0.7.4. But in this case, you would have to compare empty with non-empty iterators ;-)
3. Make html.c clean up the <table> tag better. The missing closer probably shouldn't make a difference in this example, but the segfault only happens if the closer is missing.
Since the </table> tag is mandatory, we are not obliged to handle this. Anyway, a missing </table> tag will leave a well-defined widget tree, which the selection must be able to handle. Sebastian
On Sat, Jul 12, 2003 at 04:58:42PM +0200, Sebastian Geerken wrote:
Since the </table> tag is mandatory, we are not obliged to handle this. Anyway, a missing </table> tag will leave a well-defined widget tree, which the selection must be able to handle.
While not required to handle it, the application should *never* segfault when provided bad data. -- Jamin W. Collins Linux is not The Answer. Yes is the answer. Linux is The Question. - Neo
On Sat, Jul 12, Jamin W. Collins wrote:
On Sat, Jul 12, 2003 at 04:58:42PM +0200, Sebastian Geerken wrote:
Since the </table> tag is mandatory, we are not obliged to handle this. Anyway, a missing </table> tag will leave a well-defined widget tree, which the selection must be able to handle.
While not required to handle it, the application should *never* segfault when provided bad data.
Dillo does not segfault anymore ;-) (Fix has been committed.) The problem is unrelated to the missing </table>, there may be *valid* pages which would have caused dillo to segfault due to the same bug. Sebastian
I wrote:
FYI: I seem to have fixed the bug, expect it to be committed today. Those interested should look at the differences in the code, it is only a simple coding bug, nothing very interesting.
(Has been committed.) Something I forgot: The fix does not work perfectly, when, in the test page, you start select within the text, and then move the pointer below the text, the selection starts at the beginning. This is a known bug, and it won't be fixed for 0.7.3. Sebastian
participants (3)
-
Jamin W. Collins -
Joe Crayne -
Sebastian Geerken