awslabs / amazon-kinesis-video-streams-webrtc-sdk-c

Amazon Kinesis Video Streams Webrtc SDK is for developers to install and customize realtime communication between devices and enable secure streaming of video, audio to Kinesis Video Streams.
https://awslabs.github.io/amazon-kinesis-video-streams-webrtc-sdk-c/group__PublicMemberFunctions.html
Apache License 2.0
1.03k stars 310 forks source link

[Bug]: Call serializeSessionDescriptionInit and return STATUS_BUFFER_TOO_SMALL #2019

Open liquanqing opened 3 months ago

liquanqing commented 3 months ago

Please confirm you have already done the following

Please answer the following prompt

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

disa6302 commented 3 months ago

@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?

liquanqing commented 3 months ago

@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);