After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 782216 - hostname-helper: don't read past '\0'
hostname-helper: don't read past '\0'
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-05-05 12:33 UTC by Mohammed Sadiq
Modified: 2017-05-05 13:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
hostname-helper: don't read past '\0' (833 bytes, patch)
2017-05-05 12:33 UTC, Mohammed Sadiq
none Details | Review
hostname-helper: don't read past '\0' (1.04 KB, patch)
2017-05-05 12:58 UTC, Bastien Nocera
none Details | Review
hostname-helper: don't read past '\0' (1.68 KB, patch)
2017-05-05 12:59 UTC, Mohammed Sadiq
none Details | Review
hostname-helper: don't read past '\0' (1.68 KB, patch)
2017-05-05 13:01 UTC, Mohammed Sadiq
none Details | Review

Description Mohammed Sadiq 2017-05-05 12:33:27 UTC
.
Comment 1 Mohammed Sadiq 2017-05-05 12:33:45 UTC
Created attachment 351195 [details] [review]
hostname-helper: don't read past '\0'

g_utf8_find_next_char() doesn't do checks whether the char
is '\0' or not. We have to take care of that ourself.

This commit fixes heap-buffer-overflow found by test-hostname
Comment 2 Bastien Nocera 2017-05-05 12:46:34 UTC
Review of attachment 351195 [details] [review]:

> This commit fixes heap-buffer-overflow found by test-hostname

Can you also add the warning as generated by valgrind to the commit message? Looks good otherwise.
Comment 3 Mohammed Sadiq 2017-05-05 12:53:33 UTC
(In reply to Bastien Nocera from comment #2)
> Review of attachment 351195 [details] [review] [review]:
> Can you also add the warning as generated by valgrind to the commit message?
> Looks good otherwise.

Hi the output is produced by compiling with -fsanitize=address
And the output follows, It's too long. Should I really add this to commit message:


TEST: test-hostname... (pid=18048)
  /shell/hostname:                                                     GNOME
=================================================================
==18048==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000cd76 at pc 0x7f8b26920d09 bp 0x7ffc03a9c400 sp 0x7ffc03a9c3f8
READ of size 1 at 0x60200000cd76 thread T0
OK
  /shell/ssid:                                                             #0 0x7f8b26920d08 in g_utf8_find_next_char /home/sadiq/jhbuild/checkout/glib/glib/gutf8.c:179
    #1 0x55c2b8eacaee in pretty_hostname_to_ssid /home/sadiq/jhbuild/checkout/gnome-control-center/shell/hostname-helper.c:199
    #2 0x55c2b8ead45a in test_ssid /home/sadiq/jhbuild/checkout/gnome-control-center/shell/test-hostname.c:103
    #3 0x7f8b2690ab40 in test_case_run /home/sadiq/jhbuild/checkout/glib/glib/gtestutils.c:2161
    #4 0x7f8b2690ab40 in g_test_run_suite_internal /home/sadiq/jhbuild/checkout/glib/glib/gtestutils.c:2244
    #5 0x7f8b2690ae9f in g_test_run_suite_internal /home/sadiq/jhbuild/checkout/glib/glib/gtestutils.c:2256
    #6 0x7f8b2690b122 in g_test_run_suite /home/sadiq/jhbuild/checkout/glib/glib/gtestutils.c:2332
    #7 0x7f8b2690b138 in g_test_run /home/sadiq/jhbuild/checkout/glib/glib/gtestutils.c:1599
    #8 0x55c2b8ead689 in main /home/sadiq/jhbuild/checkout/gnome-control-center/shell/test-hostname.c:135
    #9 0x7f8b262602b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
    #10 0x55c2b8eac079 in _start (/media/sadiq/Main/Software/src/gnome/gnome-control-center/shell/test-hostname+0x2079)

0x60200000cd76 is located 0 bytes to the right of 6-byte region [0x60200000cd70,0x60200000cd76)
allocated by thread T0 here:
    #0 0x7f8b2a188d28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)
    #1 0x7f8b268c9511 in g_malloc /home/sadiq/jhbuild/checkout/glib/glib/gmem.c:94
    #2 0x7f8b268fda96 in g_strndup /home/sadiq/jhbuild/checkout/glib/glib/gstrfuncs.c:425
    #3 0x7f8b26900664 in g_strsplit /home/sadiq/jhbuild/checkout/glib/glib/gstrfuncs.c:2342
    #4 0x55c2b8ead41d in test_ssid /home/sadiq/jhbuild/checkout/gnome-control-center/shell/test-hostname.c:102
    #5 0x7f8b2690ab40 in test_case_run /home/sadiq/jhbuild/checkout/glib/glib/gtestutils.c:2161
    #6 0x7f8b2690ab40 in g_test_run_suite_internal /home/sadiq/jhbuild/checkout/glib/glib/gtestutils.c:2244
    #7 0x7f8b2690ae9f in g_test_run_suite_internal /home/sadiq/jhbuild/checkout/glib/glib/gtestutils.c:2256
    #8 0x7f8b2690b122 in g_test_run_suite /home/sadiq/jhbuild/checkout/glib/glib/gtestutils.c:2332
    #9 0x7f8b2690b138 in g_test_run /home/sadiq/jhbuild/checkout/glib/glib/gtestutils.c:1599
    #10 0x55c2b8ead689 in main /home/sadiq/jhbuild/checkout/gnome-control-center/shell/test-hostname.c:135
    #11 0x7f8b262602b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/sadiq/jhbuild/checkout/glib/glib/gutf8.c:179 in g_utf8_find_next_char
Shadow bytes around the buggy address:
  0x0c047fff9950: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9960: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9970: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9980: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9990: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff99a0: fa fa fa fa fa fa fd fa fa fa 06 fa fa fa[06]fa
  0x0c047fff99b0: fa fa 01 fa fa fa 00 04 fa fa 00 00 fa fa fd fa
  0x0c047fff99c0: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fd
  0x0c047fff99d0: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa
  0x0c047fff99e0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fa
  0x0c047fff99f0: fa fa fd fd fa fa fd fa fa fa fd fa fa fa fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==18048==ABORTING
FAIL
Comment 4 Bastien Nocera 2017-05-05 12:58:16 UTC
Created attachment 351196 [details] [review]
hostname-helper: don't read past '\0'

g_utf8_find_next_char() doesn't do checks whether the char
is '\0' or not. We have to take care of that ourself.

This commit fixes heap-buffer-overflow found by test-hostname

ERROR: AddressSanitizer: heap-buffer-overflow on address
READ of size 1 at 0x60200000cd76 thread T0
 #0 0x7f8b26920d08 in g_utf8_find_next_char glib/glib/gutf8.c:179
 #1 0x55c2b8eacaee in pretty_hostname_to_ssid gnome-control-center/shell/hostname-helper.c:199
Comment 5 Mohammed Sadiq 2017-05-05 12:59:09 UTC
Created attachment 351197 [details] [review]
hostname-helper: don't read past '\0'

g_utf8_find_next_char() doesn't do checks whether the char
is '\0' or not. We have to take care of that ourself.

This commit fixes heap-buffer-overflow found by test-hostname

SUMMARY: AddressSanitizer: heap-buffer-overflow
/home/sadiq/jhbuild/checkout/glib/glib/gutf8.c:179 in g_utf8_find_next_char
Shadow bytes around the buggy address:
  0x0c047fff9950: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9960: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9970: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9980: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9990: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff99a0: fa fa fa fa fa fa fd fa fa fa 06 fa fa fa[06]fa
  0x0c047fff99b0: fa fa 01 fa fa fa 00 04 fa fa 00 00 fa fa fd fa
  0x0c047fff99c0: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fd
  0x0c047fff99d0: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa
  0x0c047fff99e0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fa
  0x0c047fff99f0: fa fa fd fd fa fa fd fa fa fa fd fa fa fa fd fd
Comment 6 Bastien Nocera 2017-05-05 13:00:38 UTC
Attachment 351197 [details] pushed as 7ca1477 - hostname-helper: don't read past '\0'

Pushed to gnome-3-22, gnome-3-24 and master
Comment 7 Mohammed Sadiq 2017-05-05 13:01:36 UTC
Created attachment 351198 [details] [review]
hostname-helper: don't read past '\0'

g_utf8_find_next_char() doesn't do checks whether the char
is '\0' or not. We have to take care of that ourself.

This commit fixes heap-buffer-overflow found by test-hostname

SUMMARY: AddressSanitizer: heap-buffer-overflow
/home/sadiq/jhbuild/checkout/glib/glib/gutf8.c:179 in g_utf8_find_next_char
Shadow bytes around the buggy address:
  0x0c047fff9950: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9960: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9970: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9980: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9990: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c047fff99a0: fa fa fa fa fa fa fd fa fa fa 06 fa fa fa[06]fa
  0x0c047fff99b0: fa fa 01 fa fa fa 00 04 fa fa 00 00 fa fa fd fa
  0x0c047fff99c0: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fd
  0x0c047fff99d0: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa
  0x0c047fff99e0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fa
  0x0c047fff99f0: fa fa fd fd fa fa fd fa fa fa fd fa fa fa fd fd
Comment 8 Mohammed Sadiq 2017-05-05 13:02:39 UTC
(In reply to Bastien Nocera from comment #6)
> Attachment 351197 [details] pushed as 7ca1477 - hostname-helper: don't read
> past '\0'
> 
> Pushed to gnome-3-22, gnome-3-24 and master

Oh. my bad, git-bz cut off some part of the commit message!