amazon-kinesis-video-streams-webrtc-sdk-c icon indicating copy to clipboard operation
amazon-kinesis-video-streams-webrtc-sdk-c copied to clipboard

[Bug]: Call serializeSessionDescriptionInit and return STATUS_BUFFER_TOO_SMALL

Open liquanqing opened this issue 1 year ago • 2 comments

Please confirm you have already done the following

  • [X] I have searched the repository for related/existing bug reports
  • [X] I have all the details the issue requires

Please answer the following prompt

  • [X] This issue is replicable using the unmodified sample application

Describe the bug

Hello, first of all, my English is poor, so please forgive me if I use inappropriate words.

When I was using the webRTC SDK, I encountered a problem that calling serializeSessionDescriptionInit returned STATUS_BUFFER_TOO_SMALL. CHK_STATUS(serializeSessionDescriptionInit(&offerSessionDescriptionInit, NULL, &buffLen)); After debugging, it was confirmed that it was caused by snprintf. I reviewed the code and found that the function logic intended to count the number of strings through snprintf, but the first time When passed in, sessionDescriptionJSON is NULL, and when actually called, the first parameter passed in is sessionDescriptionJSON + *sessionDescriptionJSONLen, but *sessionDescriptionJSONLen will accumulate, which will cause the first parameter buffer of snprintf to be passed in non-NULL and illegal parameters. , eventually causing snprintf to return -1, and amountWritten is UINT32, triggering an error when CHK(((inputSize - *sessionDescriptionJSONLen) >= amountWritten)), causing the program to end.

Expected Behavior

The function should return the serialized string length

Current Behavior

The function return STATUS_BUFFER_TOO_SMALL

Reproduction Steps

run kvsWebRTCClientViewer.exe

WebRTC C SDK version being used

1.10.2

If it was working in a previous version, which one?

No response

Compiler and Version used

gcc version 8.1.0 (x86_64-posix-seh-rev0, Built by MinGW-W64 project)

Operating System and version

Windows 10

Platform being used

minGW

liquanqing avatar Jun 26 '24 07:06 liquanqing

@liquanqing ,

Could you highlight what your application looks like? The API in our sample is invoked twice. Once to get the size:

CHK_STATUS(serializeSessionDescriptionInit(&offerSessionDescriptionInit, NULL, &buffLen));

buffLen is initialized to 0 in the viewer sample. In the function, the second parameter is NULL, which means nothing gets copied into sessionDescriptionJSON as part of SNPRINTF. The check CHK(sessionDescriptionJSON == NULL) evaluates to TRUE leading to the rest of the logic to happen. Once the size is retrieved, the next call is made:

SignalingMessage message;
CHK_STATUS(serializeSessionDescriptionInit(&offerSessionDescriptionInit, message.payload, &buffLen));

In this call, the SDK serializes the SDP.

Could you attach logs and how you are invoking the APIs in your application?

disa6302 avatar Jul 01 '24 23:07 disa6302

@disa6302 Hi, I have not modify the code in sample and just run kvsWebRTCClientViewer.exe, and it occur STATUS_BUFFER_TOO_SMALL and go to CleanUp.

The point of the question is : “ the second parameter is NULL, which means nothing gets copied into sessionDescriptionJSON as part of SNPRINTF ” This has worked in Linux or Other Platform, but in Windows with gcc version 8.1.0 (x86_64-posix-seh-rev0, Built by MinGW-W64 project), it is not work.

while ((next = STRNCHR(curr, (UINT32) (tail - curr), '\n')) != NULL) {
        lineLen = (UINT32) (next - curr);

        if (lineLen > 0 && curr[lineLen - 1] == '\r') {
            lineLen--;
        }

        amountWritten =
            SNPRINTF(sessionDescriptionJSON + *sessionDescriptionJSONLen, sessionDescriptionJSON == NULL ? 0 : inputSize - *sessionDescriptionJSONLen,
                     "%*.*s%s", lineLen, lineLen, curr, SESSION_DESCRIPTION_INIT_LINE_ENDING);
        CHK(sessionDescriptionJSON == NULL || ((inputSize - *sessionDescriptionJSONLen) >= amountWritten), STATUS_BUFFER_TOO_SMALL);

        *sessionDescriptionJSONLen += amountWritten;
        curr = next + 1;
    }

in the code, sessionDescriptionJSON is NULL, but *sessionDescriptionJSONLen will increase after "*sessionDescriptionJSONLen += amountWritten", and it will make the first parameter of SNPRINTF is "NOT NULL", firstly , the memory address is invalided. secondly, in windows, it will return -1 (UINT32_MAX), these will make the problem. the debug information as follow : error

so , i do a little change as follow and it worked :

SNPRINTF(sessionDescriptionJSON == NULL ? NULL :
sessionDescriptionJSON + *sessionDescriptionJSONLen, sessionDescriptionJSON == NULL ? 0 : inputSize - *sessionDescriptionJSONLen,
                     "%*.*s%s", lineLen, lineLen, curr, SESSION_DESCRIPTION_INIT_LINE_ENDING);

liquanqing avatar Jul 02 '24 03:07 liquanqing

This is a very old issue. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to open a new one.

github-actions[bot] avatar Jan 05 '25 00:01 github-actions[bot]