lib/barrier: Disconnect client on too long input packets

This commit is the 3/3 part of the fix for the following security
vulnerability:
 - CVE-2021-42076 DoS via excess length messages

The issue has been reported by Matthias Gerstner <mgerstner@suse.de>.
This commit is contained in:
Povilas Kanapickas 2021-11-01 05:18:53 +02:00
parent af90f39b4a
commit fd5295eb31
7 changed files with 41 additions and 7 deletions

View File

@ -17,6 +17,7 @@
*/
#include "barrier/PacketStreamFilter.h"
#include "barrier/protocol_types.h"
#include "base/IEventQueue.h"
#include "mt/Lock.h"
#include "base/TMethodEventJob.h"
@ -133,8 +134,7 @@ PacketStreamFilter::isReadyNoLock() const
return (m_size != 0 && m_buffer.getSize() >= m_size);
}
void
PacketStreamFilter::readPacketSize()
bool PacketStreamFilter::readPacketSize()
{
// note -- m_mutex must be locked on entry
@ -146,7 +146,13 @@ PacketStreamFilter::readPacketSize()
((UInt32)buffer[1] << 16) |
((UInt32)buffer[2] << 8) |
(UInt32)buffer[3];
if (m_size > PROTOCOL_MAX_MESSAGE_LENGTH) {
m_events->addEvent(Event(m_events->forIStream().inputFormatError(), getEventTarget()));
return false;
}
}
return true;
}
bool
@ -160,13 +166,17 @@ PacketStreamFilter::readMore()
UInt32 n = getStream()->read(buffer, sizeof(buffer));
while (n > 0) {
m_buffer.write(buffer, n);
// if we don't yet have the next packet size then get it, if possible.
// Note that we can't wait for whole pending data to arrive because it may be huge in
// case of malicious or erroneous peer.
if (!readPacketSize()) {
break;
}
n = getStream()->read(buffer, sizeof(buffer));
}
// if we don't yet have the next packet size then get it,
// if possible.
readPacketSize();
// note if we now have a whole packet
bool isReady = isReadyNoLock();

View File

@ -47,7 +47,9 @@ protected:
private:
bool isReadyNoLock() const;
void readPacketSize();
// returns false on erroneous packet size
bool readPacketSize();
bool readMore();
private:

View File

@ -20,6 +20,8 @@
#include "base/EventTypes.h"
#include <cstdint>
// protocol version number
// 1.0: initial protocol
// 1.1: adds KeyCode to key press, release, and repeat
@ -53,6 +55,7 @@ static const double kHeartBeatsUntilDeath = 3.0;
// Messages of very large size indicate a likely protocol error. We don't parse such messages and
// drop connection instead. Note that e.g. the clipboard messages are already limited to 32kB.
static constexpr std::uint32_t PROTOCOL_MAX_MESSAGE_LENGTH = 4 * 1024 * 1024;
static constexpr std::uint32_t PROTOCOL_MAX_LIST_LENGTH = 1024 * 1024;
static constexpr std::uint32_t PROTOCOL_MAX_STRING_LENGTH = 1024 * 1024;

View File

@ -56,6 +56,7 @@ REGISTER_EVENT(IStream, outputFlushed)
REGISTER_EVENT(IStream, outputError)
REGISTER_EVENT(IStream, inputShutdown)
REGISTER_EVENT(IStream, outputShutdown)
REGISTER_EVENT(IStream, inputFormatError)
//
// IpcClient

View File

@ -133,6 +133,11 @@ public:
*/
Event::Type outputShutdown();
/** Get input format error event type
This is sent when a stream receives an irrecoverable input format error.
*/
Event::Type inputFormatError();
//@}
private:
@ -141,6 +146,7 @@ private:
Event::Type m_outputError;
Event::Type m_inputShutdown;
Event::Type m_outputShutdown;
Event::Type m_inputFormatError;
};
class IpcClientEvents : public EventTypes {

View File

@ -51,6 +51,10 @@ ClientProxy1_0::ClientProxy1_0(const std::string& name, barrier::IStream* stream
stream->getEventTarget(),
new TMethodEventJob<ClientProxy1_0>(this,
&ClientProxy1_0::handleDisconnect, NULL));
m_events->adoptHandler(m_events->forIStream().inputFormatError(),
stream->getEventTarget(),
new TMethodEventJob<ClientProxy1_0>(this,
&ClientProxy1_0::handleDisconnect, NULL));
m_events->adoptHandler(m_events->forIStream().outputShutdown(),
stream->getEventTarget(),
new TMethodEventJob<ClientProxy1_0>(this,
@ -90,6 +94,8 @@ ClientProxy1_0::removeHandlers()
getStream()->getEventTarget());
m_events->removeHandler(m_events->forIStream().outputShutdown(),
getStream()->getEventTarget());
m_events->removeHandler(m_events->forIStream().inputFormatError(),
getStream()->getEventTarget());
m_events->removeHandler(Event::kTimer, this);
// remove timer

View File

@ -118,6 +118,10 @@ ClientProxyUnknown::addStreamHandlers()
m_stream->getEventTarget(),
new TMethodEventJob<ClientProxyUnknown>(this,
&ClientProxyUnknown::handleDisconnect));
m_events->adoptHandler(m_events->forIStream().inputFormatError(),
m_stream->getEventTarget(),
new TMethodEventJob<ClientProxyUnknown>(this,
&ClientProxyUnknown::handleDisconnect));
m_events->adoptHandler(m_events->forIStream().outputShutdown(),
m_stream->getEventTarget(),
new TMethodEventJob<ClientProxyUnknown>(this,
@ -149,6 +153,8 @@ ClientProxyUnknown::removeHandlers()
m_stream->getEventTarget());
m_events->removeHandler(m_events->forIStream().inputShutdown(),
m_stream->getEventTarget());
m_events->removeHandler(m_events->forIStream().inputFormatError(),
m_stream->getEventTarget());
m_events->removeHandler(m_events->forIStream().outputShutdown(),
m_stream->getEventTarget());
}