Ticket #164 (closed defect: fixed)

Opened 1 year ago

Last modified 1 year ago

avahi_string_list_add_vprintf uses vsnprintf incorrectly.

Reported by: QuLogic Assigned to: lennart
Priority: major Milestone: Avahi 0.6.22
Component: avahi-common Version:
Keywords: Cc:

Description

When there's some really long input, vsnprintf gets called twice with the same va_list, which doesn't work. You need to va_copy it temporarily.

Essentially, you need to apply the same change that was applied to avahi_strdup_vprintf in Changesets [472], [473], [474]. Small patch attached.

This came up in ticket #2769 in the Pidgin tracker.

Attachments

avahi-fix-vsnprintf.diff (473 bytes) - added by QuLogic on 08/25/07 04:47:17.
Fixes call to vsnprintf in avahi_string_list_add_vprintf.

Change History

08/25/07 04:47:17 changed by QuLogic

  • attachment avahi-fix-vsnprintf.diff added.

Fixes call to vsnprintf in avahi_string_list_add_vprintf.

08/25/07 04:56:45 changed by QuLogic

Oh, I meant to ask as well. Shouldn't that next line be: 

if (n >= 0 && n < (int) len + 1)

That is, add ' + 1' to the test? That's the size being passed to vsnprintf.

(follow-up: ↓ 6 ) 08/26/07 01:13:10 changed by lennart

  • status changed from new to assigned.

Regarding the +1: the bug is that "len+1" is passed to the vsnprintf() in the first place. It should be "len", because otherwise we might write past the end of the allocated string.

08/26/07 01:13:32 changed by lennart

  • milestone set to Avahi 0.6.22.

Th patch looks fine btw, will merge that now. Thank you.

08/26/07 01:14:57 changed by lennart

The irony of this issue is that the original code followed closely what is suggested in the man page vsnprintf(3) near the end. I pinged the manpages maintainer now to get him to fix this.

08/26/07 01:16:37 changed by lennart

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [1524]) properly use va_copy() when iterating more than once through a va_list; fix bad memory access by one byte; closes #164; identified by QuLogic?

(in reply to: ↑ 2 ) 08/26/07 04:10:29 changed by QuLogic

Replying to lennart:

Regarding the +1: the bug is that "len+1" is passed to the vsnprintf() in the first place. It should be "len", because otherwise we might write past the end of the allocated string.

Actually, I think "len+1" is fine. It allocates sizeof(AvahiStringList) + len where AvahiStringList includes uint8_t text[1] so you really do have "len+1" bytes available. Although, as I'm sure you've noticed, it might be a bit more confusing that way, so I'll just leave this closed.