Re: [Xen-devel] [PATCH 09/16] libelf: check nul-terminated strings properly

[Matthew Daley <mattjd@xxxxxxxxx>]

On Wed, Jun 5, 2013 at 3:15 AM, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx> wrote:
> Matthew Daley writes ("Re: [PATCH 09/16] libelf: check nul-terminated strings 
> properly"):
>> On Tue, Jun 4, 2013 at 3:41 AM, Ian Jackson <ian.jackson@xxxxxxxxxxxxx> 
>> wrote:
>> > @@ -103,10 +103,13 @@ static int print_notes(struct elf_binary *elf, 
>> > ELF_HANDLE_DECL(elf_note) start,
>> > -               if (0 != strcmp(elf_note_name(elf, note), "Xen"))
>> > +               this_note_name = elf_note_name(elf, note);
>> > +               if (NULL == this_note_name ||
>>
>> Should be break in this case, not continue, otherwise a invalid large
>> elf note section will make the loop run for a long time.
>
> I think the real problem here is the nested loop.  Perhaps we should
> impose a smallish limit on the size of strings.
>
> I've taken the view that looping once per byte of the input image is
> fine.  But I think I this loop (and others like it) can be made to
> loop O(size_of_input_image^2) times.
>
> Changing the continue to break doesn't help because if the attacker
> puts a nul byte at the end of their image they get to execute the same
> amount of loop.
>
> Ian.

I think I screwed up my explanation. I meant, if the user-supplied end
address of the note section is way off base - outside of the actual
image - elf_note_next will eventually read out-of-range 0s for the two
note sizes, and return a out-of-range handle note just past the last
one. elf_note_name will return NULL on this out-of-range note handle,
and the loop continues - maybe for up to ~0ul / 12 'notes'.

I agree with the O(n^2) problem too, though.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel