#16 expect is using memcpy where memmove is needed

None
closed-fixed
None
5
2013-11-04
2013-08-02
Per Cederqvist
No

The ecases_remove_by_expi() function in expect.c uses memcpy() to shift elements down. The areas can be overlapping. When they are, the construct is non-portable and can lead to memory corruption.

Please replace the memcpy call with a memmove call. That fixes the issue.

This is a real problem. It causes expect to randomly crash when run on the LysKOM server testsuite on an x86_64 Linux system (Ubuntu 13.04, gcc 4.7.3). valgrind helped me track down the issue.

(The issue can be reproduced by running the test suite of https://git.lysator.liu.se/lyskom-server/lyskom-server. Be sure to instrument runtest so that it uses valgrind to invoce expect.)

1 Attachments

Discussion

  • Per Cederqvist
    Per Cederqvist
    2013-11-03

    The enclosed file is a minimal example that demonstrates the issue. If you run it as "valgrind expect trigger2.expect", valgrind will report an error similar to:

    ==8467== Source and destination overlap in memcpy(0x60b9170, 0x60b9178, 16)
    ==8467==    at 0x4C2E820: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    

    The expect5.45-memmove.patch previoiusly attached to this issue fixes the problem.

     
    Attachments
    • status: open --> closed-fixed
    • assigned_to: Andreas Kupries
    • Group: -->
     
  • * exp_main_sub.c: Updated EXP_VERSION to 5.45.2
    * configure, configure.in: Updated expect to version 5.45.2
    
    * expect.c: [http://sourceforge.net/p/expect/patches/16/]. Report
      and fix both by Per Cederqvist. Replaced a memcpy with memmove
      as the latter properly handles overlapping memory, whereas the
      original code does not.
    

    Committed to CVS.
    Thank you for the report and fix.