Skip to content

fix:clientHbReq deserialize problem #34198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
guanshengliang merged 2 commits into from
Jan 8, 2026
Merged

fix:clientHbReq deserialize problem #34198

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
24d2333
add clientHbReq lenght check
Pengrongkun Jan 7, 2026
a43689b
add more check and log
Pengrongkun Jan 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 51 additions & 3 deletions source/common/src/msg/tmsg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,11 @@ static int32_t tDeserializeSClientHbReq(SDecoder *pDecoder, SClientHbReq *pReq)
int32_t line = 0;
TAOS_CHECK_RETURN(tDecodeSClientHbKey(pDecoder, &pReq->connKey));

if (pReq->connKey.connType >= CONN_TYPE__MAX || pReq->connKey.connType < 0) {
code = TSDB_CODE_INVALID_MSG;
TAOS_CHECK_GOTO(code, &line, _error);
}

if (pReq->connKey.connType == CONN_TYPE__QUERY || pReq->connKey.connType == CONN_TYPE__TMQ) {
TAOS_CHECK_RETURN(tDecodeI64(pDecoder, &pReq->app.appId));
TAOS_CHECK_RETURN(tDecodeI32(pDecoder, &pReq->app.pid));
Expand All @@ -348,7 +353,8 @@ static int32_t tDeserializeSClientHbReq(SDecoder *pDecoder, SClientHbReq *pReq)
if (queryNum) {
pReq->query = taosMemoryCalloc(1, sizeof(*pReq->query));
if (NULL == pReq->query) {
return terrno;
code = terrno;
TAOS_CHECK_GOTO(code, &line, _error);
}
code = tDecodeU32(pDecoder, &pReq->query->connId);
TAOS_CHECK_GOTO(code, &line, _error);
Expand All @@ -360,7 +366,8 @@ static int32_t tDeserializeSClientHbReq(SDecoder *pDecoder, SClientHbReq *pReq)
if (num > 0) {
pReq->query->queryDesc = taosArrayInit(num, sizeof(SQueryDesc));
if (NULL == pReq->query->queryDesc) {
return terrno;
code = terrno;
TAOS_CHECK_GOTO(code, &line, _error);
}

for (int32_t i = 0; i < num; ++i) {
Expand Down Expand Up @@ -460,6 +467,7 @@ static int32_t tDeserializeSClientHbReq(SDecoder *pDecoder, SClientHbReq *pReq)

_error:
if (code != 0) {
uError("tDeserializeSClientHbReq error, code:%d, line:%d", code, line);
tFreeClientHbReq(pReq);
}
return code;
Expand Down Expand Up @@ -558,6 +566,21 @@ int32_t tSerializeSClientHbBatchReq(void *buf, int32_t bufLen, const SClientHbBa
TAOS_CHECK_EXIT(tEncodeI32(&encoder, reqNum));
for (int32_t i = 0; i < reqNum; i++) {
SClientHbReq *pReq = taosArrayGet(pBatchReq->reqs, i);

// Calculate the serialized length of this req first by encoding to NULL buffer
SEncoder lenEncoder = {0};
tEncoderInit(&lenEncoder, NULL, 0);
int32_t code = tSerializeSClientHbReq(&lenEncoder, pReq);
int32_t reqLen = lenEncoder.pos;
tEncoderClear(&lenEncoder);
if (code < 0) {
TAOS_CHECK_EXIT(code);
}
Comment on lines +573 to +578
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The local variable code shadows the function-scoped variable of the same name. This can be confusing and lead to subtle bugs. It's better to use a different name for the local variable to improve clarity and avoid potential issues.

    int32_t ret = tSerializeSClientHbReq(&lenEncoder, pReq);
    int32_t reqLen = lenEncoder.pos;
    tEncoderClear(&lenEncoder);
    if (ret < 0) {
      TAOS_CHECK_EXIT(ret);
    }


// Encode the length before the req data
TAOS_CHECK_EXIT(tEncodeI32(&encoder, reqLen));

// Serialize the req data
TAOS_CHECK_EXIT(tSerializeSClientHbReq(&encoder, pReq));
}

Expand Down Expand Up @@ -598,8 +621,33 @@ int32_t tDeserializeSClientHbBatchReq(void *buf, int32_t bufLen, SClientHbBatchR
}
}
for (int32_t i = 0; i < reqNum; i++) {
// Read the length of this req first
int32_t reqLen = 0;
TAOS_CHECK_EXIT(tDecodeI32(&decoder, &reqLen));

if (reqLen < 0 || reqLen > decoder.size - decoder.pos) {
terrno = TSDB_CODE_INVALID_MSG;
TAOS_CHECK_EXIT(TSDB_CODE_INVALID_MSG);
}

// Create a sub-decoder limited to this req's length
SDecoder reqDecoder = {0};
reqDecoder.data = decoder.data + decoder.pos;
reqDecoder.size = reqLen;
reqDecoder.pos = 0;

SClientHbReq req = {0};
TAOS_CHECK_EXIT(tDeserializeSClientHbReq(&decoder, &req));
TAOS_CHECK_EXIT(tDeserializeSClientHbReq(&reqDecoder, &req));

// Verify that we read exactly the expected length
if (reqDecoder.pos != reqLen) {
terrno = TSDB_CODE_INVALID_MSG;
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After deserializing each individual request with tDeserializeSClientHbReq, the memory allocated within that request needs to be freed if there's an error during the verification step (line 643-645). The function tDeserializeSClientHbReq allocates memory for pReq->query and pReq->info, but if reqDecoder.pos != reqLen check fails, these resources are not cleaned up before calling TAOS_CHECK_EXIT. This can cause a memory leak since tFreeClientHbReq is only called for items already added to the array.

Suggested change
terrno = TSDB_CODE_INVALID_MSG;
terrno = TSDB_CODE_INVALID_MSG;
tFreeClientHbReq(&req);

Copilot uses AI. Check for mistakes.
TAOS_CHECK_EXIT(TSDB_CODE_INVALID_MSG);
}

// Advance the main decoder position
decoder.pos += reqLen;

if (!taosArrayPush(pBatchReq->reqs, &req)) {
TAOS_CHECK_EXIT(terrno);
}
Expand Down
Loading