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>.
(cherry picked from commit fd5295eb31
)
This commit is contained in:
parent
d762ab7d50
commit
45cd2a9f34
|
@ -17,6 +17,7 @@
|
||||||
*/
|
*/
|
||||||
|
|
||||||
#include "barrier/PacketStreamFilter.h"
|
#include "barrier/PacketStreamFilter.h"
|
||||||
|
#include "barrier/protocol_types.h"
|
||||||
#include "base/IEventQueue.h"
|
#include "base/IEventQueue.h"
|
||||||
#include "mt/Lock.h"
|
#include "mt/Lock.h"
|
||||||
#include "base/TMethodEventJob.h"
|
#include "base/TMethodEventJob.h"
|
||||||
|
@ -133,8 +134,7 @@ PacketStreamFilter::isReadyNoLock() const
|
||||||
return (m_size != 0 && m_buffer.getSize() >= m_size);
|
return (m_size != 0 && m_buffer.getSize() >= m_size);
|
||||||
}
|
}
|
||||||
|
|
||||||
void
|
bool PacketStreamFilter::readPacketSize()
|
||||||
PacketStreamFilter::readPacketSize()
|
|
||||||
{
|
{
|
||||||
// note -- m_mutex must be locked on entry
|
// note -- m_mutex must be locked on entry
|
||||||
|
|
||||||
|
@ -146,7 +146,13 @@ PacketStreamFilter::readPacketSize()
|
||||||
((UInt32)buffer[1] << 16) |
|
((UInt32)buffer[1] << 16) |
|
||||||
((UInt32)buffer[2] << 8) |
|
((UInt32)buffer[2] << 8) |
|
||||||
(UInt32)buffer[3];
|
(UInt32)buffer[3];
|
||||||
|
|
||||||
|
if (m_size > PROTOCOL_MAX_MESSAGE_LENGTH) {
|
||||||
|
m_events->addEvent(Event(m_events->forIStream().inputFormatError(), getEventTarget()));
|
||||||
|
return false;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
bool
|
bool
|
||||||
|
@ -160,12 +166,16 @@ PacketStreamFilter::readMore()
|
||||||
UInt32 n = getStream()->read(buffer, sizeof(buffer));
|
UInt32 n = getStream()->read(buffer, sizeof(buffer));
|
||||||
while (n > 0) {
|
while (n > 0) {
|
||||||
m_buffer.write(buffer, n);
|
m_buffer.write(buffer, n);
|
||||||
n = getStream()->read(buffer, sizeof(buffer));
|
|
||||||
|
// 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;
|
||||||
}
|
}
|
||||||
|
|
||||||
// if we don't yet have the next packet size then get it,
|
n = getStream()->read(buffer, sizeof(buffer));
|
||||||
// if possible.
|
}
|
||||||
readPacketSize();
|
|
||||||
|
|
||||||
// note if we now have a whole packet
|
// note if we now have a whole packet
|
||||||
bool isReady = isReadyNoLock();
|
bool isReady = isReadyNoLock();
|
||||||
|
|
|
@ -47,7 +47,9 @@ protected:
|
||||||
|
|
||||||
private:
|
private:
|
||||||
bool isReadyNoLock() const;
|
bool isReadyNoLock() const;
|
||||||
void readPacketSize();
|
|
||||||
|
// returns false on erroneous packet size
|
||||||
|
bool readPacketSize();
|
||||||
bool readMore();
|
bool readMore();
|
||||||
|
|
||||||
private:
|
private:
|
||||||
|
|
|
@ -20,6 +20,8 @@
|
||||||
|
|
||||||
#include "base/EventTypes.h"
|
#include "base/EventTypes.h"
|
||||||
|
|
||||||
|
#include <cstdint>
|
||||||
|
|
||||||
// protocol version number
|
// protocol version number
|
||||||
// 1.0: initial protocol
|
// 1.0: initial protocol
|
||||||
// 1.1: adds KeyCode to key press, release, and repeat
|
// 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
|
// 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.
|
// 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_LIST_LENGTH = 1024 * 1024;
|
||||||
static constexpr std::uint32_t PROTOCOL_MAX_STRING_LENGTH = 1024 * 1024;
|
static constexpr std::uint32_t PROTOCOL_MAX_STRING_LENGTH = 1024 * 1024;
|
||||||
|
|
||||||
|
|
|
@ -56,6 +56,7 @@ REGISTER_EVENT(IStream, outputFlushed)
|
||||||
REGISTER_EVENT(IStream, outputError)
|
REGISTER_EVENT(IStream, outputError)
|
||||||
REGISTER_EVENT(IStream, inputShutdown)
|
REGISTER_EVENT(IStream, inputShutdown)
|
||||||
REGISTER_EVENT(IStream, outputShutdown)
|
REGISTER_EVENT(IStream, outputShutdown)
|
||||||
|
REGISTER_EVENT(IStream, inputFormatError)
|
||||||
|
|
||||||
//
|
//
|
||||||
// IpcClient
|
// IpcClient
|
||||||
|
|
|
@ -133,6 +133,11 @@ public:
|
||||||
*/
|
*/
|
||||||
Event::Type outputShutdown();
|
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:
|
private:
|
||||||
|
@ -141,6 +146,7 @@ private:
|
||||||
Event::Type m_outputError;
|
Event::Type m_outputError;
|
||||||
Event::Type m_inputShutdown;
|
Event::Type m_inputShutdown;
|
||||||
Event::Type m_outputShutdown;
|
Event::Type m_outputShutdown;
|
||||||
|
Event::Type m_inputFormatError;
|
||||||
};
|
};
|
||||||
|
|
||||||
class IpcClientEvents : public EventTypes {
|
class IpcClientEvents : public EventTypes {
|
||||||
|
|
|
@ -51,6 +51,10 @@ ClientProxy1_0::ClientProxy1_0(const std::string& name, barrier::IStream* stream
|
||||||
stream->getEventTarget(),
|
stream->getEventTarget(),
|
||||||
new TMethodEventJob<ClientProxy1_0>(this,
|
new TMethodEventJob<ClientProxy1_0>(this,
|
||||||
&ClientProxy1_0::handleDisconnect, NULL));
|
&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(),
|
m_events->adoptHandler(m_events->forIStream().outputShutdown(),
|
||||||
stream->getEventTarget(),
|
stream->getEventTarget(),
|
||||||
new TMethodEventJob<ClientProxy1_0>(this,
|
new TMethodEventJob<ClientProxy1_0>(this,
|
||||||
|
@ -90,6 +94,8 @@ ClientProxy1_0::removeHandlers()
|
||||||
getStream()->getEventTarget());
|
getStream()->getEventTarget());
|
||||||
m_events->removeHandler(m_events->forIStream().outputShutdown(),
|
m_events->removeHandler(m_events->forIStream().outputShutdown(),
|
||||||
getStream()->getEventTarget());
|
getStream()->getEventTarget());
|
||||||
|
m_events->removeHandler(m_events->forIStream().inputFormatError(),
|
||||||
|
getStream()->getEventTarget());
|
||||||
m_events->removeHandler(Event::kTimer, this);
|
m_events->removeHandler(Event::kTimer, this);
|
||||||
|
|
||||||
// remove timer
|
// remove timer
|
||||||
|
|
|
@ -118,6 +118,10 @@ ClientProxyUnknown::addStreamHandlers()
|
||||||
m_stream->getEventTarget(),
|
m_stream->getEventTarget(),
|
||||||
new TMethodEventJob<ClientProxyUnknown>(this,
|
new TMethodEventJob<ClientProxyUnknown>(this,
|
||||||
&ClientProxyUnknown::handleDisconnect));
|
&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_events->adoptHandler(m_events->forIStream().outputShutdown(),
|
||||||
m_stream->getEventTarget(),
|
m_stream->getEventTarget(),
|
||||||
new TMethodEventJob<ClientProxyUnknown>(this,
|
new TMethodEventJob<ClientProxyUnknown>(this,
|
||||||
|
@ -149,6 +153,8 @@ ClientProxyUnknown::removeHandlers()
|
||||||
m_stream->getEventTarget());
|
m_stream->getEventTarget());
|
||||||
m_events->removeHandler(m_events->forIStream().inputShutdown(),
|
m_events->removeHandler(m_events->forIStream().inputShutdown(),
|
||||||
m_stream->getEventTarget());
|
m_stream->getEventTarget());
|
||||||
|
m_events->removeHandler(m_events->forIStream().inputFormatError(),
|
||||||
|
m_stream->getEventTarget());
|
||||||
m_events->removeHandler(m_events->forIStream().outputShutdown(),
|
m_events->removeHandler(m_events->forIStream().outputShutdown(),
|
||||||
m_stream->getEventTarget());
|
m_stream->getEventTarget());
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue