I think the zero-padding is necessary.
> > Concerning the array index out of bounds in=20
> nagios-3.1.0/base/utils.c:4488 I investigated a bit further.
> > Original code is:
> > 4485: /* get response */
> > 4486: recv_len=3Dsizeof(recv_buf);
> > 4487: result=3Dmy_recvall(sd,recv_buf,&recv_len,2);
> > 4488: recv_buf[sizeof(recv_buf)]=3D'\x0';
> >=20
> > Not only in 4488 a Nullbyte is written beyond the limit of=20
> recv_buf, its also zero-padding at the wrong place. After=20
> calling my_recvall recv_len contains the number of received=20
> bytes. Zero-Padding at the buffer-end would leave undefined=20
> content beginning at recv_buf[recv_len].
> > A fix should look something like this:
> > 4485: /* get response */
> > 4486: recv_len=3Dsizeof(recv_buf)-1;
> > 4487: result=3Dmy_recvall(sd,recv_buf,&recv_len,2);
> > 4488: recv_buf[recv_len]=3D'\x0';
> >=20
> > By the way: before reading anything at all my_recvall=20
> needlessly zeroes
> > out the complete recv_buf,
>=20
> Right. It shouldn't touch the buffer at all before or after=20
> writing the
> read data into it.
Line 4488 isn't useless. The buffer should be zero-padded, I'll explain =
why.
> > but when reading, it tries to fill the buffer
> > up to the end, whithout remaining space for the null-byte.
>=20
> What makes you think it should save space for the nul byte=20
> (not null, that's
> something else)? Since the size of the received data is=20
> returned as well,
> we can safely access all the data we just read. We're reading=20
> a bytestream,
> not a string. If the caller expects the bytestream to be=20
> nul-terminated,
> that's a bug in the caller, not in the routine.
Yes, it's the callers fault, my_recvall is OK.
>=20
> man 2 read
>=20
> It works the same, and it's the only sensible way it *can*=20
> work. If it all
> of a sudden starts adding nul bytes to the buffer you'll all=20
> of a sudden
> end up unable to read binary byte sequences from files and=20
> sockets alike.
Yes, all true, but then we must not use the buffer as a string.
Here zero-padding is neccessary.
* Weak argument: the debug-printf in line 4495 indicates developers=20
zero-padding intention.
* Hard argument: In line 3504 the buffer is used to call=20
get_next_string_from_buf(recv_buf,&buf_index,sizeof(recv_buf)).
While we know the number of valid bytes in the buffer, no one cares.
And even if we change this call to=20
get_next_string_from_buf(recv_buf,&buf_index,recv_len), zero-padding=20
is still necessary. In get_next_string_from_buf, line 3020, the buffer=20
is given to strcspn(buffer, "\n"). Since we can't guarantee=20
Newline-Occurence, buffer must be null-terminated or strcspn will=20
slurp beyond buffers end.
>=20
> > Things like this are the place [..]
> Please refrain from cluttering the internet with such useless=20
> flame-inciting
> statements. Especially in conjunction with such erroneous=20
> statements as
> those above.
You're right.
> [..]
Regards, Joey5337
This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]