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

[Coverity CID: 220531] Copy into fixed size buffer in tests/net/socket/misc/src/main.c #34010

Closed
zephyrbot opened this issue Apr 3, 2021 · 1 comment
Assignees
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug Coverity A Coverity detected issue or its fix priority: low Low impact/importance bug

Comments

@zephyrbot
Copy link
Collaborator

Static code scan issues found in file:

https://github.com/zephyrproject-rtos/zephyr/tree/b86f7addae05add0db45d9b528854235fbb93a48/tests/net/socket/misc/src/main.c#L245

Category: Security best practices violations
Function: test_so_bindtodevice
Component: Tests
CID: 220531

Details:

https://github.com/zephyrproject-rtos/zephyr/blob/b86f7addae05add0db45d9b528854235fbb93a48/tests/net/socket/misc/src/main.c

182    
183         /* Bind client socket with interface 1 and send a packet. */
184    
185         current_dev = NULL;
186         k_sem_reset(&send_sem);
187         strcpy(ifreq.ifr_name, dev1->name);
>>>     CID 220531:    (STRING_OVERFLOW)
>>>     You might overrun the 32-character fixed-size string "send_buf" by copying "dev1->name" without checking the length.
188         strcpy(send_buf, dev1->name);
189    
190         ret = setsockopt(sock_c, SOL_SOCKET, SO_BINDTODEVICE, &ifreq,
191                  sizeof(ifreq));
192         zassert_equal(ret, 0, "SO_BINDTODEVICE failed, %d", errno);
193    
204    
205         /* Bind client socket with interface 2 and send a packet. */
206    
207         current_dev = NULL;
208         k_sem_reset(&send_sem);
209         strcpy(ifreq.ifr_name, dev2->name);
>>>     CID 220531:    (STRING_OVERFLOW)
>>>     You might overrun the 32-character fixed-size string "send_buf" by copying "dev2->name" without checking the length.
210         strcpy(send_buf, dev2->name);
211    
212         ret = setsockopt(sock_c, SOL_SOCKET, SO_BINDTODEVICE, &ifreq,
213                  sizeof(ifreq));
214         zassert_equal(ret, 0, "SO_BINDTODEVICE failed, %d", errno);
215    
238                  sizeof(ifreq));
239         zassert_equal(ret, 0, "SO_BINDTODEVICE failed, %d", errno);
240    
241         /* Bind client socket with interface 1 again. */
242    
243         k_sem_reset(&send_sem);
>>>     CID 220531:    (STRING_OVERFLOW)
>>>     You might overrun the 48-character fixed-size string "ifreq.ifr_name" by copying "dev1->name" without checking the length.
244         strcpy(ifreq.ifr_name, dev1->name);
245         strcpy(send_buf, dev1->name);
246    
247         ret = setsockopt(sock_c, SOL_SOCKET, SO_BINDTODEVICE, &ifreq,
248                  sizeof(ifreq));
249         zassert_equal(ret, 0, "SO_BINDTODEVICE failed, %d", errno);
203         k_msleep(10);
204    
205         /* Bind client socket with interface 2 and send a packet. */
206    
207         current_dev = NULL;
208         k_sem_reset(&send_sem);
>>>     CID 220531:    (STRING_OVERFLOW)
>>>     You might overrun the 48-character fixed-size string "ifreq.ifr_name" by copying "dev2->name" without checking the length.
209         strcpy(ifreq.ifr_name, dev2->name);
210         strcpy(send_buf, dev2->name);
211    
212         ret = setsockopt(sock_c, SOL_SOCKET, SO_BINDTODEVICE, &ifreq,
213                  sizeof(ifreq));
214         zassert_equal(ret, 0, "SO_BINDTODEVICE failed, %d", errno);
181         zassert_equal(ret, 0, "SO_BINDTODEVICE failed, %d", errno);
182    
183         /* Bind client socket with interface 1 and send a packet. */
184    
185         current_dev = NULL;
186         k_sem_reset(&send_sem);
>>>     CID 220531:    (STRING_OVERFLOW)
>>>     You might overrun the 48-character fixed-size string "ifreq.ifr_name" by copying "dev1->name" without checking the length.
187         strcpy(ifreq.ifr_name, dev1->name);
188         strcpy(send_buf, dev1->name);
189    
190         ret = setsockopt(sock_c, SOL_SOCKET, SO_BINDTODEVICE, &ifreq,
191                  sizeof(ifreq));
192         zassert_equal(ret, 0, "SO_BINDTODEVICE failed, %d", errno);
172    
173         ret = bind(sock_s, bind_addr, bind_addrlen);
174         zassert_equal(ret, 0, "bind failed, %d", errno);
175    
176         /* Bind server socket with interface 2. */
177    
>>>     CID 220531:    (STRING_OVERFLOW)
>>>     You might overrun the 48-character fixed-size string "ifreq.ifr_name" by copying "dev2->name" without checking the length.
178         strcpy(ifreq.ifr_name, dev2->name);
179         ret = setsockopt(sock_s, SOL_SOCKET, SO_BINDTODEVICE, &ifreq,
180                  sizeof(ifreq));
181         zassert_equal(ret, 0, "SO_BINDTODEVICE failed, %d", errno);
182    
183         /* Bind client socket with interface 1 and send a packet. */
239         zassert_equal(ret, 0, "SO_BINDTODEVICE failed, %d", errno);
240    
241         /* Bind client socket with interface 1 again. */
242    
243         k_sem_reset(&send_sem);
244         strcpy(ifreq.ifr_name, dev1->name);
>>>     CID 220531:    (STRING_OVERFLOW)
>>>     You might overrun the 32-character fixed-size string "send_buf" by copying "dev1->name" without checking the length.
245         strcpy(send_buf, dev1->name);
246    
247         ret = setsockopt(sock_c, SOL_SOCKET, SO_BINDTODEVICE, &ifreq,
248                  sizeof(ifreq));
249         zassert_equal(ret, 0, "SO_BINDTODEVICE failed, %d", errno);
250    

Please fix or provide comments in coverity using the link:

https://scan9.coverity.com/reports.htm#v29271/p12996

Note: This issue was created automatically. Priority was set based on classification
of the file affected and the impact field in coverity. Assignees were set using the CODEOWNERS file.

@zephyrbot zephyrbot added bug The issue is a bug, or the PR is fixing a bug Coverity A Coverity detected issue or its fix priority: low Low impact/importance bug labels Apr 3, 2021
@rlubos
Copy link
Contributor

rlubos commented Apr 7, 2021

False-positive, the device names used in the test were finetuned to fit into the provided buffers.

@rlubos rlubos closed this as completed Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug Coverity A Coverity detected issue or its fix priority: low Low impact/importance bug
Projects
None yet
Development

No branches or pull requests

5 participants