--- a/nxcomp/ClientChannel.cpp +++ b/nxcomp/ClientChannel.cpp @@ -447,6 +447,26 @@ } } + // Get other bits of the header, so will not need to refer to them again + unsigned char inputDataByte = inputMessage[1]; + unsigned int buffer2 = GetUINT(inputMessage + 2, bigEndian_); + unsigned int inputDataSize = buffer2 - 1; + if (buffer2 == 0) + { + // BIG-REQUESTS + inputMessage += 4; + inputLength -= 4; + inputDataSize = GetULONG(inputMessage, bigEndian_) - 2; + } + if (inputLength != (4 * (inputDataSize + 1))) + { + #ifdef WARNING + *logofs << "handleRead: inputLength=" << inputLength + << " mismatch inputDataSize=" << inputDataSize + << ".\n" << logofs_flush; + #endif + } + // // Go to the message's specific encoding. // @@ -501,8 +521,36 @@ encodeBuffer.encodeCachedValue(format, 8, clientCache_ -> changePropertyFormatCache); unsigned int dataLength = GetULONG(inputMessage + 20, bigEndian_); + + // Self-preserving sanity check (otherwise we crash and dump core): + // some clients do this when not getting their beloved BIG-REQUESTS. + unsigned int maxLength = 0; + if (format == 8) + { + maxLength = inputLength - 24; + } + else if (format == 32) + { + maxLength = (inputLength - 24) >> 2; + } + else if (format == 16) + { + maxLength = (inputLength - 24) >> 1; + } + if (dataLength > maxLength) + { + #ifdef WARNING + *logofs << "ChangeProperty bogus dataLength=" << dataLength + << " set to " << maxLength + << " when format=" << (int)format + << " inputLength=" << inputLength + << ".\n" << logofs_flush; + #endif + dataLength = maxLength; + } + encodeBuffer.encodeValue(dataLength, 32, 6); - encodeBuffer.encodeValue(inputMessage[1], 2); + encodeBuffer.encodeValue(inputDataByte, 2); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> windowCache); encodeBuffer.encodeCachedValue(GetULONG(inputMessage + 8, bigEndian_), 29, @@ -533,7 +581,7 @@ nextSrc += 4; } } - else + else if (format == 16) { for (unsigned int i = 0; i < dataLength; i++) { @@ -541,6 +589,13 @@ nextSrc += 2; } } + else + { + #ifdef WARNING + *logofs << "ChangeProperty bogus format=" << (int)format + << ".\n" << logofs_flush; + #endif + } } break; case X_SendEvent: @@ -562,7 +617,7 @@ break; } - encodeBuffer.encodeBoolValue((unsigned int) inputMessage[1]); + encodeBuffer.encodeBoolValue((unsigned int) inputDataByte); unsigned int window = GetULONG(inputMessage + 4, bigEndian_); if (window == 0 || window == 1) @@ -599,7 +654,7 @@ break; case X_ChangeWindowAttributes: { - encodeBuffer.encodeValue((inputLength - 12) >> 2, 4); + encodeBuffer.encodeValue(inputDataSize - 2, 4); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> windowCache); unsigned int bitmask = GetULONG(inputMessage + 8, bigEndian_); @@ -654,7 +709,7 @@ break; } - encodeBuffer.encodeBoolValue((unsigned int) inputMessage[1]); + encodeBuffer.encodeBoolValue((unsigned int) inputDataByte); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> windowCache); const unsigned char *nextSrc = inputMessage + 8; @@ -1000,12 +1055,12 @@ { #ifdef TARGETS - *logofs << "handleRead: X_CreatePixmap depth " << (unsigned) inputMessage[1] + *logofs << "handleRead: X_CreatePixmap depth " << (unsigned) inputDataByte << ", pixmap id " << GetULONG(inputMessage + 4, bigEndian_) << ", drawable " << GetULONG(inputMessage + 8, bigEndian_) << ", width " << GetUINT(inputMessage + 12, bigEndian_) << ", height " << GetUINT(inputMessage + 14, bigEndian_) - << ", size " << GetUINT(inputMessage + 2, bigEndian_) << 2 + << ", length " << inputLength << ".\n" << logofs_flush; unsigned int p_id = GetULONG(inputMessage + 4, bigEndian_); @@ -1054,7 +1109,7 @@ #endif unsigned bitmask = GetULONG(inputMessage + 28, bigEndian_); - encodeBuffer.encodeCachedValue((unsigned int) inputMessage[1], 8, + encodeBuffer.encodeCachedValue((unsigned int) inputDataByte, 8, clientCache_ -> depthCache); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 8, bigEndian_), clientCache_ -> windowCache); @@ -1138,7 +1193,7 @@ break; } - unsigned int numPoints = ((inputLength - 16) >> 2); + unsigned int numPoints = (inputDataSize - 3); if (control -> isProtoStep10() == 1) { @@ -1209,7 +1264,7 @@ break; case X_FreeColors: { - unsigned int numPixels = GetUINT(inputMessage + 2, bigEndian_) - 3; + unsigned int numPixels = inputDataSize - 2; encodeBuffer.encodeValue(numPixels, 16, 4); encodeBuffer.encodeCachedValue(GetULONG(inputMessage + 4, bigEndian_), 29, clientCache_ -> colormapCache); @@ -1378,7 +1433,7 @@ break; } - encodeBuffer.encodeBoolValue((unsigned int) inputMessage[1]); + encodeBuffer.encodeBoolValue((unsigned int) inputDataByte); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> windowCache); unsigned int property = GetULONG(inputMessage + 8, bigEndian_); @@ -1404,7 +1459,7 @@ break; case X_GrabButton: { - encodeBuffer.encodeBoolValue((unsigned int) inputMessage[1]); + encodeBuffer.encodeBoolValue((unsigned int) inputDataByte); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> windowCache); encodeBuffer.encodeCachedValue(GetUINT(inputMessage + 8, bigEndian_), 16, @@ -1423,7 +1478,7 @@ break; case X_GrabPointer: { - encodeBuffer.encodeBoolValue((unsigned int) inputMessage[1]); + encodeBuffer.encodeBoolValue((unsigned int) inputDataByte); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> windowCache); encodeBuffer.encodeCachedValue(GetUINT(inputMessage + 8, bigEndian_), 16, @@ -1448,7 +1503,7 @@ break; case X_GrabKeyboard: { - encodeBuffer.encodeBoolValue((unsigned int) inputMessage[1]); + encodeBuffer.encodeBoolValue((unsigned int) inputDataByte); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> windowCache); unsigned int timestamp = GetULONG(inputMessage + 8, bigEndian_); @@ -1673,7 +1728,7 @@ break; } - unsigned int textLength = (unsigned int) inputMessage[1]; + unsigned int textLength = (unsigned int) inputDataByte; encodeBuffer.encodeCachedValue(textLength, 8, clientCache_ -> imageTextLengthCache, 4); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, @@ -1740,7 +1795,7 @@ break; } - unsigned int textLength = (unsigned int) inputMessage[1]; + unsigned int textLength = (unsigned int) inputDataByte; encodeBuffer.encodeCachedValue(textLength, 8, clientCache_ -> imageTextLengthCache, 4); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, @@ -1797,7 +1852,7 @@ unsigned int nameLength = GetUINT(inputMessage + 4, bigEndian_); encodeBuffer.encodeValue(nameLength, 16, 6); - encodeBuffer.encodeBoolValue((unsigned int) inputMessage[1]); + encodeBuffer.encodeBoolValue((unsigned int) inputDataByte); const unsigned char *nextSrc = inputMessage + 8; if (control -> isProtoStep7() == 1) @@ -2269,8 +2324,8 @@ break; } - encodeBuffer.encodeValue(GetUINT(inputMessage + 2, bigEndian_) - 3, 16, 4); - encodeBuffer.encodeBoolValue((unsigned int) inputMessage[1]); + encodeBuffer.encodeValue(inputDataSize - 2, 32, 4); + encodeBuffer.encodeBoolValue((unsigned int) inputDataByte); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> drawableCache); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 8, bigEndian_), @@ -2336,8 +2391,8 @@ break; } - encodeBuffer.encodeValue(GetUINT(inputMessage + 2, bigEndian_) - 3, 16, 4); - encodeBuffer.encodeBoolValue((unsigned int) inputMessage[1]); + encodeBuffer.encodeValue(inputDataSize - 2, 32, 4); + encodeBuffer.encodeBoolValue((unsigned int) inputDataByte); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> drawableCache); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 8, @@ -2370,8 +2425,7 @@ break; case X_PolyRectangle: { - encodeBuffer.encodeValue((GetUINT(inputMessage + 2, - bigEndian_) - 3) >> 1, 16, 3); + encodeBuffer.encodeValue((inputDataSize - 2) >> 1, 32, 3); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> drawableCache); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 8, @@ -2424,8 +2478,7 @@ break; } - encodeBuffer.encodeValue((GetUINT(inputMessage + 2, - bigEndian_) - 3) >> 1, 16, 4); + encodeBuffer.encodeValue((inputDataSize - 2) >> 1, 32, 4); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> drawableCache); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 8, @@ -2522,7 +2575,7 @@ break; case X_QueryBestSize: { - encodeBuffer.encodeValue((unsigned int)inputMessage[1], 2); + encodeBuffer.encodeValue((unsigned int)inputDataByte, 2); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> drawableCache); encodeBuffer.encodeValue(GetUINT(inputMessage + 8, bigEndian_), 16, 8); @@ -2538,7 +2591,7 @@ // Differential encoding. encodeBuffer.encodeBoolValue(1); - unsigned int numColors = ((inputLength - 8) >> 2); + unsigned int numColors = (inputDataSize - 1); encodeBuffer.encodeValue(numColors, 16, 5); encodeBuffer.encodeCachedValue(GetULONG(inputMessage + 4, bigEndian_), 29, clientCache_ -> colormapCache); @@ -2636,7 +2689,7 @@ break; } - unsigned int numRectangles = ((inputLength - 12) >> 3); + unsigned int numRectangles = ((inputDataSize - 2) >> 1); if (control -> isProtoStep9() == 1) { @@ -2647,7 +2700,7 @@ encodeBuffer.encodeValue(numRectangles, 13, 4); } - encodeBuffer.encodeValue((unsigned int) inputMessage[1], 2); + encodeBuffer.encodeValue((unsigned int) inputDataByte, 2); encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> gcCache); encodeBuffer.encodeCachedValue(GetUINT(inputMessage + 8, bigEndian_), 16, @@ -2802,7 +2855,7 @@ } // Format. - encodeBuffer.encodeValue((unsigned int) inputMessage[1], 2); + encodeBuffer.encodeValue((unsigned int) inputDataByte, 2); // Drawable. encodeBuffer.encodeXidValue(GetULONG(inputMessage + 4, bigEndian_), clientCache_ -> drawableCache); @@ -3004,7 +3057,7 @@ << ".\n" << logofs_flush; #endif - encodeBuffer.encodeCachedValue(*(inputMessage + 1), 8, + encodeBuffer.encodeCachedValue(inputDataByte, 8, clientCache_ -> resourceCache); } else if (inputOpcode == opcodeStore_ -> freeUnpack) @@ -3015,7 +3068,7 @@ << ".\n" << logofs_flush; #endif - encodeBuffer.encodeCachedValue(*(inputMessage + 1), 8, + encodeBuffer.encodeCachedValue(inputDataByte, 8, clientCache_ -> resourceCache); } else if (inputOpcode == opcodeStore_ -> getControlParameters) @@ -3198,10 +3251,10 @@ { if (hit) { - statistics -> addRenderCachedRequest(*(inputMessage + 1)); + statistics -> addRenderCachedRequest(inputDataByte); } - statistics -> addRenderRequestBits(*(inputMessage + 1), inputLength << 3, bits); + statistics -> addRenderRequestBits(inputDataByte, inputLength << 3, bits); } } // End if (firstRequest_)... else ... --- a/nxcomp/ClientReadBuffer.cpp +++ b/nxcomp/ClientReadBuffer.cpp @@ -119,15 +119,32 @@ dataLength = (GetUINT(start + 2, bigEndian_) << 2); - if (dataLength < 4) + if (dataLength == 0) // or equivalently (dataLength < 4) { - #ifdef TEST - *logofs << "ClientReadBuffer: WARNING! Assuming length 4 " - << "for suspicious message of length " << dataLength - << ".\n" << logofs_flush; - #endif + // BIG-REQUESTS extension + if (size < 8) + { + remaining_ = 8 - size; + return 0; + } - dataLength = 4; + dataLength = (GetULONG(start + 4, bigEndian_) << 2); + + if (dataLength < 8 || dataLength > 1024*1024*1024) + { + #ifdef WARNING + *logofs << "BIG-REQUESTS with unacceptable dataLength=" + << dataLength << ", now set to 8.\n" << logofs_flush; + #endif + dataLength = 8; + } + else if (dataLength < 4*64*1024) + { + #ifdef WARNING + *logofs << "BIG-REQUESTS with silly dataLength=" + << dataLength << ".\n" << logofs_flush; + #endif + } } } --- a/nxcomp/ServerChannel.cpp +++ b/nxcomp/ServerChannel.cpp @@ -104,7 +104,8 @@ // #define HIDE_MIT_SHM_EXTENSION -#define HIDE_BIG_REQUESTS_EXTENSION +// HIDE_BIG_REQUESTS_EXTENSION : No good to hide, some clients may send crap instead... +#undef HIDE_BIG_REQUESTS_EXTENSION #define HIDE_XFree86_Bigfont_EXTENSION #undef HIDE_SHAPE_EXTENSION #undef HIDE_XKEYBOARD_EXTENSION @@ -3756,7 +3757,7 @@ } unsigned int numPoints; - decodeBuffer.decodeValue(numPoints, 16, 4); + decodeBuffer.decodeValue(numPoints, 32, 4); outputLength = (numPoints << 2) + 12; outputMessage = writeBuffer_.addMessage(outputLength); unsigned int relativeCoordMode; @@ -3802,7 +3803,7 @@ } unsigned int numPoints; - decodeBuffer.decodeValue(numPoints, 16, 4); + decodeBuffer.decodeValue(numPoints, 32, 4); outputLength = (numPoints << 2) + 12; outputMessage = writeBuffer_.addMessage(outputLength); unsigned int relativeCoordMode; @@ -3839,7 +3840,7 @@ case X_PolyRectangle: { unsigned int numRectangles; - decodeBuffer.decodeValue(numRectangles, 16, 3); + decodeBuffer.decodeValue(numRectangles, 32, 3); outputLength = (numRectangles << 3) + 12; outputMessage = writeBuffer_.addMessage(outputLength); decodeBuffer.decodeXidValue(value, clientCache_ -> drawableCache); @@ -3869,7 +3870,7 @@ } unsigned int numSegments; - decodeBuffer.decodeValue(numSegments, 16, 4); + decodeBuffer.decodeValue(numSegments, 32, 4); outputLength = (numSegments << 3) + 12; outputMessage = writeBuffer_.addMessage(outputLength); decodeBuffer.decodeXidValue(value, clientCache_ -> drawableCache); @@ -4590,7 +4591,21 @@ *outputMessage = (unsigned char) outputOpcode; - PutUINT(outputLength >> 2, outputMessage + 2, bigEndian_); + if (outputLength < 4*64*1024) + PutUINT(outputLength >> 2, outputMessage + 2, bigEndian_); + else + { + // Handle BIG-REQUESTS + PutUINT(0, outputMessage + 2, bigEndian_); +// BUG ALERT: following write may not work well, +// particularly with un-flushed messages. +// But, it works well enough in my testing... + // Write first four bytes + if (transport_ -> write(write_immediate, outputMessage, 4) < 0) + return -1; + // Replace with new 4-byte length + PutULONG(1 + (outputLength >> 2), outputMessage, bigEndian_); + } #if defined(TEST) || defined(OPCODES) *logofs << "handleWrite: Handled request OPCODE#"