Comment 10 for bug 1459692

I reviewed anope version 2.0.6-1 as checked into cosmic.
This shouldn't be considered a full audit but rather a quick gauge of
maintainability.

Anope is a set of services for IRC networks. It allow users/admins to
manage their nicks/channels/networks and more.
Quick list of services:
 - NickServ
 - ChanServ
 - MemoServ
 - BotServ
 - OperServ
 - HostServ

- No CVEs registered against anope.
- Build-depends:
 - debhelper (>= 10),
 - cmake,
 - default-libmysqlclient-dev,
 - libldap2-dev,
 - libpcre3-dev,
 - libgnutls28-dev,
 - libsqlite3-dev
- postinst and post/pre rm automatically added
- init script: /etc/init.d/anope
  - Has a: chown irc /var/run/anope (not recursive)
- No systemd services
- No dbus services
- No setuid bit
- Binaries in PATH: /usr/sbin/anope
- No sudo fragments
- No udev rules
- No tests
- No cron jobs
- Some lintian warning/error. The permission warning I would ignore,
0700 permission looks better than 0755 for db backups.
  E: anope changes: bad-distribution-in-changes-file unstable
  W: anope: non-standard-dir-perm var/lib/anope/db/backups/ 0700 !=
0755
  W: anope: binary-without-manpage usr/sbin/anope
  N: 12 tags overridden (12 warnings)

- Lack of input sanitization:
  ./modules/extra/m_regex_pcre.cpp:36: return pcre_exec(this->regex,
NULL, str.c_str(), str.length(), 0, 0, NULL, 0) > -1;
  ./modules/extra/m_regex_tre.cpp:38: return regexec(&this->regbuf,
str.c_str(), 0, NULL, 0) == 0;
  ./modules/extra/m_regex_posix.cpp:37: return regexec(&this->regbuf,
str.c_str(), 0, NULL, 0) == 0;

  None of those regex engines do input sanitization, and there is no
sanitization on anope's code. We reported it to upstream, see more
information at the bottom of this comment.

- Processes spawned:
  ./src/main.cpp:212: execve(Anope::ServicesBin.c_str(), av, envp);
  ./src/config.cpp:681: this->fp = (this->executable ? popen(this-
>name.c_str(), "r") : fopen((Anope::ConfigDir + "/" + this-
>name).c_str(), "r"));
  ./src/mail.cpp:30: FILE *pipe = popen(sendmail_path.c_str(), "w");

  Although they look dangerous, we understood that the input come from Anope's
configuration file, which is under administrator control, so probably fine.

- There are many file IO operations and memory management operations in
the project. After spending some time I couldn't find any trivial way
to trigger an overflow/underflow, but more time would be required in
order to be truly sure.

- Logging looks ok

- Make use of the following environment languages: LANGUAGE e LANG.
Looks safe.
./src/language.cpp:104: setenv("LANG", lang, 1);
./src/language.cpp:105: setenv("LANGUAGE", lang, 1);
./src/language.cpp:115: unsetenv("LANGUAGE");
./src/language.cpp:116: unsetenv("LANG");

- Anope make use of the following privileged functions. All of them are
used in the same function setuidgid(), which is executed during Anope's
initialization.
The setgid and setuid will only be triggered if the user specifies a
specific user and group on anope's config file. The chown will be
executed on every initialization to set the owner of the log files to
either the specified user (if defined in the config file) or to the
current user that is running Anope.
./src/init.cpp:272: if (setgid(gid) == -1)
./src/init.cpp:279: if (setuid(uid) == -1)
./src/init.cpp:266: chown(lf->filename.c_str(),
uid, gid);

  We certainly didn't love that chown, but since Ubuntu has YAMA loaded
it is probably safe. We wonder if this could be a problem in kernels
without YAMA.

- Anope implements MD5, SHA1, SHA256 and BLOWFISH in
modules/encryption/, those modules are used when dealing with passwords
so it stores the password in the databases securely. Another way to
authenticate users can be done by using sasl and ldap modules found on
modules/extras/m_sasl or modules/extras/m_ldap.
Anope also has two modules for SSL/TLS: modules/extras/m_ssl_openssl
and modules/extras/m_ssl_gnutls. Both of them provide SSL services to
Anope using either openssl and gnutls.

- Extensive networking. Didn't check all of them, just a couple and
they looked safe.
- No WebKit
- No PolicyKit
- The build log has some warnings:

/<<PKGBUILDDIR>>/src/init.cpp:109:9: warning: ignoring return value of
‘FILE* freopen(const char*, const char*, FILE*)’, declared with
attribute warn_unused_result [-Wunused-result]

/<<PKGBUILDDIR>>/src/init.cpp:110:9: warning: ignoring return value of
‘FILE* freopen(const char*, const char*, FILE*)’, declared with
attribute warn_unused_result [-Wunused-result]

/<<PKGBUILDDIR>>/src/init.cpp:111:9: warning: ignoring return value of
‘FILE* freopen(const char*, const char*, FILE*)’, declared with
attribute warn_unused_result [-Wunused-result]

> Those seem ok, nothing catastrophic but would be nice to see those warnings solved.

/<<PKGBUILDDIR>>/src/init.cpp:266:9: warning: ignoring return value of
‘int chown(const char*, __uid_t, __gid_t)’, declared with attribute
warn_unused_result [-Wunused-result]

/<<PKGBUILDDIR>>/src/main.cpp:209:8: warning: ignoring return value of
‘int chdir(const char*)’, declared with attribute warn_unused_result [-
Wunused-result]

> The previous two look more dangerous. If the chdir() fails it still could execute a file from the current working directory.

/<<PKGBUILDDIR>>/src/misc.cpp:742:60: warning: ‘void* memcpy(void*,
const void*, size_t)’ copying an object of non-trivial type ‘union
sockaddrs’ from an array of ‘struct sockaddr’ [-Wclass-memaccess]

/<<PKGBUILDDIR>>/src/pipeengine.cpp:60:7: warning: ignoring return
value of ‘ssize_t write(int, const void*, size_t)’, declared with
attribute warn_unused_result [-Wunused-result]

> Probably fine, but still could fix this for a clean build log.

/<<PKGBUILDDIR>>/modules/m_dns.cpp:955:57: warning: ?: using integer
constants in boolean context, the expression will always evaluate to
‘true’ [-Wint-in-bool-context]

> This should be really checked and fixed.

- I also got some warnings from shellcheck, nothing to be worried
but I've also reported it to upstream.

So we reported all those issues we saw here:
https://bugs.anope.org/view.php?id=1717

The bug is right now private and I am unable to change this, so I will be
updating as I get some reply from them (no replies since last Tuesday).

Before giving our (Security team) decision on this MIR, I would like to hear back from upstream.

If there're any doubts just let me know.