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.

Sign up for GitHub

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

Jump to bottom

segfault in slist_delete() if list contains only one element #20

Closed
LeSpocky opened this issue May 9, 2016 · 0 comments · Fixed by #22
Closed

segfault in slist_delete() if list contains only one element #20

LeSpocky opened this issue May 9, 2016 · 0 comments · Fixed by #22
Labels
bug
Milestone
v1.1

Comments

@LeSpocky
Copy link
Collaborator

LeSpocky commented May 9, 2016

If the list contains only one element the following line segfaults:

if (!strcasecmp((*start)->next->name, name)) {

With only one element (*start)->next is NULL and there's no check for that → segfault.
@LeSpocky LeSpocky added the bug label May 9, 2016
@LeSpocky LeSpocky added this to the v1.1 milestone May 9, 2016
LeSpocky added a commit to LeSpocky/libcgi that referenced this issue May 9, 2016
@LeSpocky
list delete: return if next element is null (fixes rafaelsteil#20)
a4f2e93 
Signed-off-by: Alexander Dahl <post@lespocky.de>
@LeSpocky LeSpocky mentioned this issue May 9, 2016
Merged
@LeSpocky LeSpocky mentioned this issue May 9, 2016
Closed
LeSpocky added a commit to LeSpocky/libcgi that referenced this issue Feb 24, 2017
@LeSpocky
list delete: return if next element is null (fixes rafaelsteil#20)
20670fd 
Signed-off-by: Alexander Dahl <post@lespocky.de>
LeSpocky added a commit to LeSpocky/libcgi that referenced this issue Mar 17, 2017
@LeSpocky
Don't allocate string buffers smaller than sizeof(void *)
dc3c94e 
When reviewing rafaelsteil#34 valgrind threw errors on some tests on a amd64
machine with CMAKE_BUILD_TYPE=Release or RelWithDebInfo:

% ctest -R valgrind_process_form --output-on-failure                                                            :(
Test project /home/alex/build/libcgi
    Start 20: valgrind_process_form
1/1 Test rafaelsteil#20: valgrind_process_form ............***Failed    0.88 sec
==5017== Memcheck, a memory error detector
==5017== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==5017== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==5017== Command: /home/alex/build/libcgi/test/cgi-test process_form
==5017==
==5017== Invalid read of size 4
==5017==    at 0x4E37E6D: cgi_unescape_special_chars (cgi.c:472)
==5017==    by 0x4E38056: process_data (cgi.c:132)
==5017==    by 0x4E38248: cgi_process_form (cgi.c:171)
==5017==    by 0x4021ED: test_cgi_process_form (test.c:209)
==5017==    by 0x401043: main (test.c:44)
==5017==  Address 0x53e83d0 is 0 bytes inside a block of size 2 alloc'd
==5017==    at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==5017==    by 0x4E37DE6: cgi_unescape_special_chars (cgi.c:438)
==5017==    by 0x4E38056: process_data (cgi.c:132)
==5017==    by 0x4E38248: cgi_process_form (cgi.c:171)
==5017==    by 0x4021ED: test_cgi_process_form (test.c:209)
==5017==    by 0x401043: main (test.c:44)
==5017==
==5017== Invalid read of size 4
==5017==    at 0x4E37E6D: cgi_unescape_special_chars (cgi.c:472)
==5017==    by 0x4E38056: process_data (cgi.c:132)
==5017==    by 0x4E3827D: cgi_process_form (cgi.c:200)
==5017==    by 0x402402: _post (test.c:175)
==5017==    by 0x402402: test_cgi_process_form (test.c:226)
==5017==    by 0x401043: main (test.c:44)
==5017==  Address 0x53e8d50 is 0 bytes inside a block of size 2 alloc'd
==5017==    at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==5017==    by 0x4E37DE6: cgi_unescape_special_chars (cgi.c:438)
==5017==    by 0x4E38056: process_data (cgi.c:132)
==5017==    by 0x4E3827D: cgi_process_form (cgi.c:200)
==5017==    by 0x402402: _post (test.c:175)
==5017==    by 0x402402: test_cgi_process_form (test.c:226)
==5017==    by 0x401043: main (test.c:44)
==5017==
test_cgi_process_form
==5017==
==5017== HEAP SUMMARY:
==5017==     in use at exit: 0 bytes in 0 blocks
==5017==   total heap usage: 26 allocs, 26 frees, 2,006 bytes allocated
==5017==
==5017== All heap blocks were freed -- no leaks are possible
==5017==
==5017== For counts of detected and suppressed errors, rerun with: -v
==5017== ERROR SUMMARY: 4 errors from 2 contexts (suppressed: 0 from 0)

0% tests passed, 1 tests failed out of 1

Total Test time (real) =   0.88 sec

The following tests FAILED:
         20 - valgrind_process_form (Failed)
Errors while running CTest

The invalid read is done by an optimized version of strlen() which reads
4 byte from a buffer which was allocated with a size of 2 byte. You can
see some details on this behaviour for other cases here:

https://bugs.launchpad.net/ubuntu/+source/valgrind/+bug/852760

While it is possible to silence those warnings from valgrind with the
option --partial-loads-ok=yes I thought it would not make much sense to
allocate buffers smaller than sizeof(void *) because malloc would
allocate aligned chunks anyway and in the case of 2 bytes the not
allocated bytes were wasted. With this patch we would assume some
optimized functions would operate on the string buffers created by
various libcgi functions anyway, but prevent them to access memory which
we didn't really allocate.
LeSpocky added a commit to LeSpocky/libcgi that referenced this issue Mar 17, 2017
@LeSpocky
Don't allocate string buffers smaller than sizeof(void *)
792b3de 
When reviewing rafaelsteil#34 valgrind threw errors on some tests on a amd64
machine with CMAKE_BUILD_TYPE=Release or RelWithDebInfo:

% ctest -R valgrind_process_form --output-on-failure                                                            :(
Test project /home/alex/build/libcgi
    Start 20: valgrind_process_form
1/1 Test rafaelsteil#20: valgrind_process_form ............***Failed    0.88 sec
==5017== Memcheck, a memory error detector
==5017== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==5017== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==5017== Command: /home/alex/build/libcgi/test/cgi-test process_form
==5017==
==5017== Invalid read of size 4
==5017==    at 0x4E37E6D: cgi_unescape_special_chars (cgi.c:472)
==5017==    by 0x4E38056: process_data (cgi.c:132)
==5017==    by 0x4E38248: cgi_process_form (cgi.c:171)
==5017==    by 0x4021ED: test_cgi_process_form (test.c:209)
==5017==    by 0x401043: main (test.c:44)
==5017==  Address 0x53e83d0 is 0 bytes inside a block of size 2 alloc'd
==5017==    at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==5017==    by 0x4E37DE6: cgi_unescape_special_chars (cgi.c:438)
==5017==    by 0x4E38056: process_data (cgi.c:132)
==5017==    by 0x4E38248: cgi_process_form (cgi.c:171)
==5017==    by 0x4021ED: test_cgi_process_form (test.c:209)
==5017==    by 0x401043: main (test.c:44)
==5017==
==5017== Invalid read of size 4
==5017==    at 0x4E37E6D: cgi_unescape_special_chars (cgi.c:472)
==5017==    by 0x4E38056: process_data (cgi.c:132)
==5017==    by 0x4E3827D: cgi_process_form (cgi.c:200)
==5017==    by 0x402402: _post (test.c:175)
==5017==    by 0x402402: test_cgi_process_form (test.c:226)
==5017==    by 0x401043: main (test.c:44)
==5017==  Address 0x53e8d50 is 0 bytes inside a block of size 2 alloc'd
==5017==    at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==5017==    by 0x4E37DE6: cgi_unescape_special_chars (cgi.c:438)
==5017==    by 0x4E38056: process_data (cgi.c:132)
==5017==    by 0x4E3827D: cgi_process_form (cgi.c:200)
==5017==    by 0x402402: _post (test.c:175)
==5017==    by 0x402402: test_cgi_process_form (test.c:226)
==5017==    by 0x401043: main (test.c:44)
==5017==
test_cgi_process_form
==5017==
==5017== HEAP SUMMARY:
==5017==     in use at exit: 0 bytes in 0 blocks
==5017==   total heap usage: 26 allocs, 26 frees, 2,006 bytes allocated
==5017==
==5017== All heap blocks were freed -- no leaks are possible
==5017==
==5017== For counts of detected and suppressed errors, rerun with: -v
==5017== ERROR SUMMARY: 4 errors from 2 contexts (suppressed: 0 from 0)

0% tests passed, 1 tests failed out of 1

Total Test time (real) =   0.88 sec

The following tests FAILED:
         20 - valgrind_process_form (Failed)
Errors while running CTest

The invalid read is done by an optimized version of strlen() which reads
4 byte from a buffer which was allocated with a size of 2 byte. You can
see some details on this behaviour for other cases here:

https://bugs.launchpad.net/ubuntu/+source/valgrind/+bug/852760

While it is possible to silence those warnings from valgrind with the
option --partial-loads-ok=yes I thought it would not make much sense to
allocate buffers smaller than sizeof(void *) because malloc would
allocate aligned chunks anyway and in the case of 2 bytes the not
allocated bytes were wasted. With this patch we would assume some
optimized functions would operate on the string buffers created by
various libcgi functions anyway, but prevent them to access memory which
we didn't really allocate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
v1.1
Development

Successfully merging a pull request may close this issue.

list delete: fix two segfaults LeSpocky/libcgi
1 participant
@LeSpocky