Hi Mike,
Thank you for the review!
On Wed, Aug 6, 2014 at 7:14 PM, Mike Frysinger <vap...@gentoo.org> wrote:
> On Mon 04 Aug 2014 08:31:30 zubin.mit...@gmail.com wrote:
>> --- a/bjm.c
>> +++ b/bjm.c
>> @@ -121,6 +121,8 @@ sys_query_module(struct tcb *tcp)
>> (idx ? ", " :
>> ""),
>> mod);
>> mod += strlen(mod)+1;
>> + if (mod-data >=
>> tcp->u_arg[3])
>> + break;
>
> this check is incomplete. if the buffer is not NUL terminated, then the
> string reading code will read beyond the end. you can protect against that by
> doing:
> char *data = malloc(tcp->u_arg[3] + 1);
> then before the for loop here, make sure the buffer is NUL terminated:
> data[tcp->u_arg[3]] = '\0';
>
> as for this check, i think it's weirdly written. it makes more sense to me:
> if (mod >= data + tcp->u_arg[3])
>
> also, when you do break, don't you want to print out a ... to indicate ?
I've made these changes.
>
>> @@ -144,6 +146,8 @@ sys_query_module(struct tcb *tcp)
>> tprintf(" /* %lu entries */ ",
>> (unsigned long)ret);
>> } else {
>> for (idx = 0; idx < ret;
>> idx++) {
>> + if ((long)sym->name >=
>> tcp->u_arg[3])
>> + break;
>> tprintf("%s{name=%s,
>> value=%lu}",
>> (idx ? " " :
>> ""),
>>
>> data+(long)sym->name,
>
> i think this too is incomplete. the sym++ might walk past the end of the
> data, sym->name is unsigned while u_arg is signed, and the symbol name might
> start near the end of the valid region but then walk beyond it (no NUL
> termination). so i guess what are you trying to protect against ?
The idea was to make sure that a dereference of "data+(long)sym->name"
inside tprintf(upon processing "%s") should not lead to an access of
an invalid address(i.e. the address at data+(long)sym->name needs to
be valid). However, the check I had added in the patch fails to do
that. Would there be a way to do that check elegantly?
I'll add a bounds check to sym in the loop.
Thanks,
-- zm
------------------------------------------------------------------------------
Infragistics Professional
Build stunning WinForms apps today!
Reboot your WinForms applications with our WinForms controls.
Build a bridge from your legacy apps to the future.
http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel
