RE: [AD] 3 patches regarding unicode and buffer overfows (mostly)

[ Thread Index | Date Index | More lists.liballeg.org/allegro-developers Archives ]


> I tried to review the 3 patches and I made some corrections (surrounded in
> the original patches), but as it is the first time I seriously cope with
> Unicode stuff, I may have actually screwed them up a little...
> Could Annie take a look at the revised patches and correct my 
> corrections ?

Here are her answer. Sorry for the delay, but we don't have an Internet
connection at home, so I courrier via floppy :)
It seems she mainly got caught by ustrn* using bytes rather than unicode
characters :P

-- 
Lyrian 
* ascii.diff

==========================
after recent modifications, get_config_string() can't return NULL or the empty string if 'def' is set.
==========================
Good

========================
safer to use external buffer.
========================
You're right


* fbcon.diff

===========================
I disagree: n is the number of bytes appended to the end of fb_desc.
===========================
Oops! I should have read the doc...


* nolimit.diff

patch -p1 < nolimit.diff

==================================
we modify file_select_ex() instead to accept the size parameter
==================================
Fine


================================
I disagree with your 'last<SIZE_OF_ARRAY(tok)' test: the last loop may be executed with last = SIZE_OF_ARRAY(tok)-1,
hence at the end of the loop: last = SIZE_OF_ARRAY(tok) then tok[last] = ustrtok(NULL, toks) is out of bound.
And 'last<SIZE_OF_ARRAY(tok)-1' may lose one token.
================================
You're right


===========================
I disagree: n is the number of bytes appended to the end of s.
===========================
Oops! I should have read the doc...


==========================
I disagree: the length must be in characters.
==========================
Since there is a call to ustrncpy(dest, buf, size-ucwidth(0)) in both
fix_filename_path and replace_filename, the size parameter should be in bytes
(or does these functions have changed?)
And I think that using a temp variable to contain the size in bytes
(my 'raw_size' variable) simplify the code.


=========================
Why not ustrsize() ?
=========================
Because i was tired ?


Annie



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/