Skip to content
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.

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

[Bug]: Call serializeSessionDescriptionInit and return STATUS_BUFFER_TOO_SMALL #2019

Open
3 tasks done
liquanqing opened this issue Jun 26, 2024 · 2 comments
Open
3 tasks done
Labels
bug Something isn't working v1.10.2

Comments

@liquanqing
Copy link

Please confirm you have already done the following

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

Please answer the following prompt

  • 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 liquanqing added bug Something isn't working needs-triage labels Jun 26, 2024
@disa6302
Copy link
Contributor

disa6302 commented Jul 1, 2024

@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
Copy link
Author

liquanqing commented Jul 2, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v1.10.2
Projects
None yet
Development

No branches or pull requests

3 participants