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

Fix overflow in memcpy #2859

Closed
wants to merge 2 commits into from
Closed

Conversation

npes87184
Copy link
Contributor

@npes87184 npes87184 commented Dec 4, 2021

In function ‘memcpy’,                             
    inlined from ‘control_msg_serialize.constprop’ at ../app/src/control_msg.c:77:5,
    inlined from ‘run_controller’ at ../app/src/controller.c:69:12:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:34:10: warning: ‘__builtin___memcpy_chk’ writing 262138 bytes into a region of
 size 262130 overflows the destination [-Wstringop-overflow=]
   return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
          ^

When serialize CONTROL_MSG_TYPE_SET_CLIPBOARD event, it did not consider the size of sequence which is 8 bytes. So that the buffer size is not enough for memcpy.

Signed-off-by: Yu-Chen Lin <npes87184@gmail.com>
@npes87184 npes87184 changed the title Fix build warning Fix overflow in memcpy Dec 4, 2021
In function ‘memcpy’,
    inlined from ‘control_msg_serialize.constprop’ at ../app/src/control_msg.c:77:5,
    inlined from ‘run_controller’ at ../app/src/controller.c:69:12:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:34:10: warning: ‘__builtin___memcpy_chk’ writing 262138 bytes into a region of
 size 262130 overflows the destination [-Wstringop-overflow=]
   return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));

Signed-off-by: Yu-Chen Lin <npes87184@gmail.com>
@rom1v
Copy link
Collaborator

rom1v commented Dec 4, 2021

LGTM 👍

How did you detect it in practice? It may occur only on very specific text size...

@npes87184
Copy link
Contributor Author

npes87184 commented Dec 4, 2021

It was detected by compiler. 😉

@rom1v
Copy link
Collaborator

rom1v commented Dec 4, 2021

Which compiler? I tested with both gcc and clang, I don't get this warning. Any specific options?

rom1v pushed a commit that referenced this pull request Dec 4, 2021
In function ‘memcpy’,
    inlined from ‘control_msg_serialize.constprop’ at ../app/src/control_msg.c:77:5,
    inlined from ‘run_controller’ at ../app/src/controller.c:69:12:
        /usr/include/x86_64-linux-gnu/bits/string_fortified.h:34:10:
        warning: ‘__builtin___memcpy_chk’ writing 262138 bytes into a region
        of size 262130 overflows the destination [-Wstringop-overflow=]
   return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));

Refs 901d837
PR #2859 <#2859>

Signed-off-by: Yu-Chen Lin <npes87184@gmail.com>
Signed-off-by: Romain Vimont <rom@rom1v.com>
rom1v added a commit that referenced this pull request Dec 4, 2021
This would have catched the possible memcpy() overflow fixed by the
previous commit.

Refs #2859 <#2859>
@rom1v
Copy link
Collaborator

rom1v commented Dec 4, 2021

Merged:

I added a unit test: ae90ef2

@rom1v rom1v closed this Dec 4, 2021
@npes87184
Copy link
Contributor Author

Which compiler? I tested with both gcc and clang, I don't get this warning. Any specific options?

I use gcc.

The Meson build system
Version: 0.55.3
Source dir: /home/yclin/scrcpy
Build dir: /home/yclin/scrcpy/x
Build type: native build
Project name: scrcpy
Project version: 1.21
C compiler for the host machine: cc (gcc 7.5.0 "cc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0")
C linker for the host machine: cc ld.bfd 2.30
Host machine cpu family: x86_64
Host machine cpu: x86_64
Found pkg-config: /usr/bin/pkg-config (0.29.1)
Run-time dependency libavformat found: YES 57.83.100
Run-time dependency libavcodec found: YES 57.107.100
Run-time dependency libavutil found: YES 55.78.100
Run-time dependency sdl2 found: YES 2.0.17
Run-time dependency libavdevice found: YES 57.10.100
Run-time dependency libusb-1.0 found: YES 1.0.21
Checking for function "strdup" : YES 
Checking for function "asprintf" : YES 
Checking for function "vasprintf" : YES 
Header <sys/socket.h> has symbol "SOCK_CLOEXEC" : YES 
Configuring config.h using configuration
Program ./scripts/build-wrapper.sh found: YES
Build targets in project: 3

Found ninja-1.8.2 at /usr/bin/ninja

The detection option is -Wstringop-overflow=.

Would it relate to the meson?

@npes87184 npes87184 deleted the fix_build_warning branch December 4, 2021 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants