I reviewed ubuntuone-credentials 13.08-0ubuntu1 from the NEW queue for
saucy. This should not be considered a full security audit, but rather a
quick gauge of maintainability.

- ubuntuone-credentials provides an interface for native and QML
  applications to use and manage Ubuntu One credentials
- Build-deps include liboauth, libaccounts-qt5, libsignon-qt5, qtbase5,
  qtdeclarative5, qtdeclarative5-ubuntu-ui-toolkid-plugin, signon-plugins,
  and xvfb (presumably for testing)
- Relies upon Qt to provide networking and SSL/TLS of connections
- Relies upon liboauth for authentication tokens, signing, etc.
- Does not daemonize
- Does not listen on the network
- No initscripts
- No D-Bus services
- No setuid
- No binaries in *bin/
- No sudo fragments
- No cronjobs
- Clean build logs
- Good test suite, runs during build
  - Unclear if error messages are expected
- No subprocesses spawned
- Memory management is slightly leaky but nothing that should pose a
  reliability or safety issue, all are intended to be long-lived objects
  with currently no mechanism to "un-use" the objects.
- Files opened are for logging, using high-level Qt abstractions
- Paths to files are determined by Qt helper routines and environment
  variables
- Logging operations look safe
- Environment handling looks safe
- No privileged portions of code
- Certificate validation handled by Qt, no listeners are installed to
  ignore SSL/TLS errors.
  - But also looks like "pinning" certificates in the future would be
    difficult
- Network traffic is parsed with Qt json parsing code, only explicit
  fields are retrieved
  - But I'm unsure how invalid json documents affect code execution
- Does not directly use WebKit, though Qml does
- Even though hardening-check reports problems, I believe they are false
  positives -- -D_FORTIFY_SOURCE=2 appears everywhere, and the stack check
  problem may be because there are no protectable on-stack arrays in the
  libubuntuoneplugin library.

Two notes that I hope are useful:

AuthLogger::stopLogging() race condition may delete an in-use logger in
another thread. If multi-threaded use is possible, this module likely
needs some additional work.

music-login/main.cpp main() forgets to close stylesheet file.


I have two questions:

1: Why does Keyring::deleteToken() only delete one account?

2. How will the code respond to json documents that don't have all
specified fields? Will applications that make use of this package expect
the data to have any particular formatting or contents that should be
enforced?


Security team ACK for including in ubuntu-touch images. Please consult
with the security team again in the future when this package is intended
to move to main.

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1199017

Title:
  [needs-packaging] ubuntuone-credentials

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntuone-credentials/+bug/1199017/+subscriptions

-- 
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to