Menu

#221 TiLP doesn't Latin (non-ASCII) chars correctly

open
nobody
None
5
2016-11-01
2014-10-31
Tom Li
No

When I tried to backup all the apps on my TI-84, TiLP crashed because of a strcpy() overflow.

Program received signal SIGABRT, Aborted.
0x00007ffff694bdf7 in raise () from /lib64/libc.so.6
(gdb) bt

0 0x00007ffff694bdf7 in raise () from /lib64/libc.so.6

1 0x00007ffff694d168 in abort () from /lib64/libc.so.6

2 0x00007ffff698cd70 in __libc_message () from /lib64/libc.so.6

3 0x00007ffff6a148f7 in __fortify_fail () from /lib64/libc.so.6

4 0x00007ffff6a128d0 in __chk_fail () from /lib64/libc.so.6

5 0x00007ffff7b86e62 in strcpy (src=0xa94800 "Français", dest=0xa8244c "Français") at /usr/include/bits/string3.h:104

6 recv_flash (handle=0x47dab0, content=0xa82440, vr=0xa94400)

at /mnt/vm/compile/portage/sci-libs/libticalcs2-1.1.8/work/libticalcs2-1.1.8/src/calc_84p.c:455

7 0x00007ffff7b7a1ca in ticalcs_calc_recv_tigroup (handle=handle@entry=0x47dab0, content=content@entry=0x99d340, mode=mode@entry=TIG_FLASH)

at /mnt/vm/compile/portage/sci-libs/libticalcs2-1.1.8/work/libticalcs2-1.1.8/src/backup.c:338

8 0x00007ffff7b7c7bf in ticalcs_calc_recv_tigroup2 (handle=0x47dab0, filename=0x9000e0 "/tmp/tilp.tigroup", mode=TIG_FLASH)

at /mnt/vm/compile/portage/sci-libs/libticalcs2-1.1.8/work/libticalcs2-1.1.8/src/calc_xx.c:1859

9 0x000000000040e489 in _start ()

I created a patch for libtifiles2

diff -uprN /tmp/libtifiles2-1.1.6.orig/src/tifiles.h /tmp/libtifiles2-1.1.6/src/tifiles.h
--- /tmp/libtifiles2-1.1.6.orig/src/tifiles.h 2014-10-31 21:50:22.141971904 +0800
+++ /tmp/libtifiles2-1.1.6/src/tifiles.h 2014-10-31 21:57:39.241667003 +0800
@@ -240,7 +240,7 @@ typedef struct
uint8_t revision_day;
uint8_t revision_month;
uint16_t revision_year;
- char name[9];
+ char name[VARNAME_MAX];
uint8_t device_type;
uint8_t data_type;
uint32_t data_length;
@@ -301,7 +301,7 @@ struct _FlashContent
uint8_t revision_day;
uint8_t revision_month;
uint16_t revision_year;
- char name[9];
+ char name[VARNAME_MAX];
uint8_t device_type;
uint8_t data_type;
uint8_t hw_id;

Now it is no longer crashes, but the backup still failed.

(tilp:26870): tifiles-CRITICAL : string passed in 'write_string8' is too long (>n chars).
(tilp:26870): tifiles-CRITICAL
: s = Français, len(s) = 9
tifiles-INFO: 46 72 61 6E C3 A7 61 69 73 (9)
(tilp:26870): tifiles-CRITICAL **: ti8x_file_write_flash: error writing file /tmp/tigCY9HOX.8Xk

I realized that the "ç" in string "Français" is the source of the issue.

$ ./test_length

strlen("Francais") == 8
strlen("Français") == 9
strlen("çççççççç") == 16

Since the maximum length for app names is 8 chars, we assumed all strings of the app names are char[8], but it is not correct. 8 characters != sizeof(char * 8). We need to rework on all the code related to char[8], maybe replace char to w_char is a solution.

Discussion

  • Tom Li

    Tom Li - 2014-10-31

    It's my first time to use SourceForge. I didn't know that SourceForge supports Markdown, sorry to the formatting :(

    The title is also broken because of a typo, It is not editable :(

     

    Last edit: Tom Li 2014-10-31
  • Tom Li

    Tom Li - 2014-10-31

    currently, I am using a dirty hack

    diff -uprN /tmp/libticalcs2-1.1.8.orig/src/calc_84p.c     /tmp/libticalcs2-1.1.8/src/calc_84p.c
    --- /tmp/libticalcs2-1.1.8.orig/src/calc_84p.c  2014-10-31 22:37:29.433414304 +0800
    +++ /tmp/libticalcs2-1.1.8/src/calc_84p.c       2014-10-31 22:38:11.762681907 +0800
    @@ -452,7 +452,8 @@ static int      recv_flash  (CalcHandle* hand
        TRYF(dusb_cmd_r_var_content(handle, NULL, &data));
    
        content->model = handle->model;
    -   strcpy(content->name, vr->name);
    +   strncpy(content->name, vr->name, 9);
    +   content->name[8] = '\0';
        content->data_type = vr->type;
        content->device_type = DEVICE_TYPE_83P;
        content->num_pages = 2048;  // TI83+ has 512 KB of FLASH max
    

    which drops the extra chars. But

    • the length of name[] (9) is a private state for libtifiles2,
    • it causes the incorrect apps name in backup
    • all calc_*.c need modification.

    this is a real dirty hack.

     

    Last edit: Tom Li 2015-05-18
  • Lionel Debroux

    Lionel Debroux - 2014-10-31

    Thanks for the report. It happens that this bug, in the same function, was already reported on an IRC chan several weeks ago :)
    Locally, I've precisely started eliminating all strcpy and strcat calls from the code base, because it needs to be done anyway, irrespective of fixing other encoding-related issues.
    Switching from char to wchar is not an option with UTF-8.

     
  • Lionel Debroux

    Lionel Debroux - 2016-11-01

    Fixed a while ago in Git, the fix is now part of the libraries associated to TILP II 1.18.

     

Log in to post a comment.