Re: [libvirt] [PATCH] phyp: Fixing possible buffer overflow

(this new patch is corrupt and doesn't apply. Not sure how you sent it, but I'm guessing your mailer mangled it. It's usually better to only send patches with git send-email, or to attach the output of git format-patch to the mail, rather than pasting it inline)

This version is closer *in location* to avoiding the overflow, but it's still comparing the wrong index - it's looking at the index into the full array (ret, ie "i") rather than into the small copy that is used to convert (id_c, ie "j"). Also, it still allows writing past the end of the array (i == j == 4, so it can write to id_c[4]). Even allowing a write to id_c[3] would be a problem, since that would overwrite the terminating NULL, thus creating the possibility that virStrToLong_i could overrun the end of the array (or, if virStrToLong_i fails, the VIR_ERROR following it would try to print a string that is not NULL terminated - a *much* worse prospect). Additionally the 2nd occurence of "memset(id_c, 0, 10)" inside the loop that I noted in my last mail has been left unchanged (and this memset will be done every time the function is called, even if there is no overflow of string length, so it is *guaranteed* the buffer will be overflowed). Finally, in the case that the output of the exec is too long, this new patch will simply exit from the loop early and return success, rather than logging an error and returning failure. These problems *could* be fixed by 1) changing "i <= 4" to "j < 3", 2) fixing the 2nd memset to clear 4 bytes instead of 10, and 3) turning the case of "j >= 3" into an error instead of just exiting the loop. However... did you look at the counter-proposed patch in my previous email? I'm assuming that either you didn't notice it, or that you're trying to come up with a patch that makes the fewest changes possible while fixing the perceived bug. I think that patch would be a better idea for several reasons, and as long as we're fixing this function, we may as well fix it in the best way possible.