Limit max username/password size in SSecurityPlain.
Setting the limit to 1024 which should be still more than enough.
Unlimited ulen and plen can cause various security problems:
* Overflow in `is->checkNoWait(ulen + plen)` causing it to contine when there is not enough data and then wait forever.
* Overflow in `new char[plen + 1]` that would allocate zero sized array which succeeds but returns pointer that should not be written into.
* Allocation failure in `new char[plen + 1]` from trying to allocate too much and crashing the whole server.
All those issues can be triggered by a client before authentication.
diff --git a/common/rfb/SSecurityPlain.cxx b/common/rfb/SSecurityPlain.cxx
index 0531549..fc9dff2 100644
--- a/common/rfb/SSecurityPlain.cxx
+++ b/common/rfb/SSecurityPlain.cxx
@@ -86,8 +86,15 @@
if (state == 0) {
if (!is->checkNoWait(8))
return false;
+
ulen = is->readU32();
+ if (ulen > MaxSaneUsernameLength)
+ throw AuthFailureException("Too long username");
+
plen = is->readU32();
+ if (plen > MaxSanePasswordLength)
+ throw AuthFailureException("Too long password");
+
state = 1;
}
diff --git a/common/rfb/SSecurityPlain.h b/common/rfb/SSecurityPlain.h
index 080fcd5..2c08c24 100644
--- a/common/rfb/SSecurityPlain.h
+++ b/common/rfb/SSecurityPlain.h
@@ -54,6 +54,9 @@
PasswordValidator* valid;
unsigned int ulen, plen, state;
CharArray username;
+
+ static const unsigned int MaxSaneUsernameLength = 1024;
+ static const unsigned int MaxSanePasswordLength = 1024;
};
}