Comment 5 for bug 1997106

Revision history for this message
Rodrigo Figueiredo Zaiden (rodrigo-zaiden) wrote :

I reviewed mosh 1.4.0-1 as checked into lunar. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

mosh (Mobile Shell) is a remote terminal application that replaces
interactive SSH terminals. mosh supports intermittent connectivity and
claims to be more robust and responsive over lagging networks.

- CVE History:
  - CVE-2012-2385, was solved in a few days.
  - upstream seems pretty responsive, as already stated.
- Build-Depends?
  - debhelper-compat, protobuf-compiler, libprotobuf-dev, pkg-config,
    libutempter-dev, zlib1g-dev, libncurses5-dev, libssl-dev,
    bash-completion, locales, tmux, less (all in main)
- pre/post inst/rm scripts?
  - pre/post inst/rm scripts removes mosh file in bash_completion.d/
     `rm_conffile /etc/bash_completion.d/mosh 1.2.5\~ -- "$@"`.
- init scripts?
  - NA
- systemd units?
  - NA
- dbus services?
  - NA
- setuid binaries?
  - NA
- binaries in PATH?
  - mosh, mosh-client, mosh-server
- sudo fragments?
  - NA
- polkit files?
  - NA
- udev rules?
  - NA
- unit tests / autopkgtests?
  - There is a testsuite that runs during the build process with 31 tests,
    the tests are well documented in src/tests/README.md.
  - No autopkgtests.
- cron jobs?
  - NA
- Build logs:
  - Build log is clean.

- Processes spawned?
  - bash, shell and locale are being spawned in some places. they were
    reviewed and does not seem problematic. places spawning bash/shell are
    using execvp and locale invoked by system does not have interaction
    with user input, so it can't be exploited.
- Memory management?
  - it seems fine, tried to check all flawfinder reports and in general
    they look good.
- File IO?
  - most are standard i/o as used in a terminal application, all fine.
- Logging?
  - flawfinder pointed to some potential format string problems, but they
    where checked and are safe, the variables that could cause overflows
    are checked before their usage in logs.
- Environment variable usage?
  - use of env variable for general configuration and to pass the key, if
    needed (by mosh-client). seems well parsed and filtered.
- Use of privileged functions?
  - use of ioctls, mainly to get terminal window size.
- Use of cryptography / random number sources etc?
  - Uses OpenSSL AES-OCB implementation for secure network communication
    with 16bytes key length through OpenSSL EVP API.
- Use of temp files?
  - NA
- Use of networking?
  - seems well defined and implemented, nothing concerning.
- Use of WebKit?
  - NA
- Use of PolicyKit?
  - NA

- Any significant cppcheck results?
  - NA
- Any significant Coverity results?
  - There is a large stack usage being reported twice:
     src/frontend/stmclient.cc:306
     src/frontend/mosh-server.cc:835
    that is the buffer that handles the terminal input, having it as big as
    defined is part of the usability as I see.
- Any significant shellcheck results?
  - No
- Any significant bandit results?
  - NA

The project is well written and well documented.
Upstream seems active and have history of security awareness: there is a
security contact in mosh.org and there is history of Coverity runs in the
past.

Security team ACK for promoting mosh to main. As already stated as TODO,
it would be nice to see autopkgtests included before main inclusion.