Skip to content

Commit

Permalink
Limit max username/password size in SSecurityPlain.
Browse files Browse the repository at this point in the history
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 + 2)` 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.
  • Loading branch information
michalsrb committed Mar 29, 2017
1 parent e317573 commit 761b2aa
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 0 deletions.
7 changes: 7 additions & 0 deletions common/rfb/SSecurityPlain.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
3 changes: 3 additions & 0 deletions common/rfb/SSecurityPlain.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

}
Expand Down

0 comments on commit 761b2aa

Please sign in to comment.