[MIR] librelp

[Michael Terry]

A dependency of the (desired) new default system logger, rsyslog.

See https://wiki.ubuntu.com/MainInclusionReport/librelp

[Kees Cook (kees)]

relpOffersToString does not bounds-check the output string (even has a "TODO" listed), as it uses a fixed 4096 size. Once this is fixed, I can approve the MIR.

[Michael Terry (mterry)]

I emailed the author, Rainer Gerhards. He said this:

"I've had a quick look at the code. It looks indeed like an easy fix, but I
think there is no issue at all (thus the TODO is not yet done): as far as I
remember, this is only called from within the RELP application and not based
on anything received from the wire. So it can not be exploited, because the
current RELP code never generates a greeting of that size (it less than 512
bytes). But I will check tomorrow in more detail."

He hasn't gotten back to me yet in a couple days, so I assume no further surprises appeared. I've sent a follow up.

As for where the function is used... It's not exposed as part of the UI, but it is in the symbols table. It's used twice in the source, but I'm not qualified to tell if they're safe uses myself. It would seem to depend on how long the 'offers' array is.

[Michael Terry (mterry)]

I asked:
"Any news? So you're saying that the array of offers is guaranteed to be
small in the two usages of the function in RELP?"

And Rainer replied:
"Yes, for the current version. The offers are generated based on capabilities
and the current code has not enough capabilities to exhaust the buffer.
Anyhow, I'll look at it as soon as I am finished with my rsyslog threading
work. Probably the best cure is count the size and do a realloc() if it is
exhausted. That safes it for future development (it's too easy to forget
about fixing it once other things are developed...).

None of the offers is user-provided, though, so even in this case I don't see
a way to exploit it (except crashing on its own, which is kind of a DoS...)."

[Kees Cook (kees)]

Yeah, walking the packet receiver, ToString appears safe for the moment. I'm worried about this code growing and gaining more functionality. While re-reviewing, I also see that relpOfferValueAdd will wrap integers (since Data len is 255 characters, converted back to int), though nothing meaningfully depends on this yet. If an intVal is ever used for length calculates, there will be trouble. (Also note strncpy doesn't terminate if it encounters max characters, though again, currently safe due to equal sized src/dest buffers.)

+1 since this is blocking rsyslog, but we should carefully watch this package.

[Colin Watson (cjwatson)]

OK, I'll promote this to main for rsyslog, thanks. Perhaps somebody could file a separate bug about the problems identified here?

[Colin Watson (cjwatson)]

2009-07-17 21:33:33 INFO creating lockfile
2009-07-17 21:33:37 INFO Override Component to: 'main'
2009-07-17 21:33:37 INFO 'librelp - 0.1.3-1/universe/libs' source overridden
2009-07-17 21:33:37 INFO 'librelp-dev-0.1.3-1/universe/libdevel/OPTIONAL' binary overridden in karmic/amd64
2009-07-17 21:33:37 INFO 'librelp-dev-0.1.3-1/universe/libdevel/OPTIONAL' binary overridden in karmic/armel
2009-07-17 21:33:37 INFO 'librelp-dev-0.1.3-1/universe/libdevel/OPTIONAL' binary overridden in karmic/i386
2009-07-17 21:33:37 INFO 'librelp-dev-0.1.3-1/universe/libdevel/OPTIONAL' binary overridden in karmic/ia64
2009-07-17 21:33:37 INFO 'librelp-dev-0.1.3-1/universe/libdevel/OPTIONAL' binary overridden in karmic/lpia
2009-07-17 21:33:37 INFO 'librelp-dev-0.1.3-1/universe/libdevel/OPTIONAL' binary overridden in karmic/powerpc
2009-07-17 21:33:37 INFO 'librelp-dev-0.1.3-1/universe/libdevel/OPTIONAL' binary overridden in karmic/sparc
2009-07-17 21:33:37 INFO 'librelp0-0.1.3-1/universe/libs/OPTIONAL' binary overridden in karmic/amd64
2009-07-17 21:33:37 INFO 'librelp0-0.1.3-1/universe/libs/OPTIONAL' binary overridden in karmic/armel
2009-07-17 21:33:37 INFO 'librelp0-0.1.3-1/universe/libs/OPTIONAL' binary overridden in karmic/i386
2009-07-17 21:33:37 INFO 'librelp0-0.1.3-1/universe/libs/OPTIONAL' binary overridden in karmic/ia64
2009-07-17 21:33:37 INFO 'librelp0-0.1.3-1/universe/libs/OPTIONAL' binary overridden in karmic/lpia
2009-07-17 21:33:37 INFO 'librelp0-0.1.3-1/universe/libs/OPTIONAL' binary overridden in karmic/powerpc
2009-07-17 21:33:38 INFO 'librelp0-0.1.3-1/universe/libs/OPTIONAL' binary overridden in karmic/sparc
Confirm this transaction? [yes, no] yes
2009-07-17 21:34:57 INFO Transaction committed.
2009-07-17 21:34:57 INFO Done.