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 05315490b9e15c77affbec041d27e43e3b5256df..fc9dff23eab8b95546f97671ff8359bdf8f1b2e0 100644 (file)
--- a/common/rfb/SSecurityPlain.cxx
+++ b/common/rfb/SSecurityPlain.cxx
@@ -86,8 +86,15 @@ bool SSecurityPlain::processMsg(SConnection* sc)
   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 080fcd598f631b1714175c3233c2d420d34ce86c..2c08c24e6566e5844d7aca64f2da5f4a57439378 100644 (file)
--- a/common/rfb/SSecurityPlain.h
+++ b/common/rfb/SSecurityPlain.h
@@ -54,6 +54,9 @@ namespace rfb {
     PasswordValidator* valid;
     unsigned int ulen, plen, state;
     CharArray username;
+
+    static const unsigned int MaxSaneUsernameLength = 1024;
+    static const unsigned int MaxSanePasswordLength = 1024;
   };
 
 }