Hello, I reviewed mecab version 0.996-6 as checked into cosmic. This is
not a full security audit but rather a quick gauge of maintainability.
- One CVE for mecab in our CVE database.
- mecab is a japanese natural language parser
- Build-Depends: debhelper
- Does not daemonize
- Does not do networking
- postinst looks autogenerated
- No initscript
- No systemd unit files
- No dbus services
- No setuid
- mecab-config and mecab binaries in path
- No sudo fragments
- No udev rules
- Some tests run during the build, I didn't investigate their depth
- No cronjobs
- Relatively clean build logs
- No subprocesses spawned
- Memory management is way too manual; I didn't spot any errors but this
code would benefit from a C++14-aware rewrite.
- Logging looked good
- Environment variables HOME and MECABRC used, looked fine
- No privileged operations
- No cryptography
- No networking
- No privileged portions of code
- No temporary files
- No WebKit
- No JavaScript
- No PolicyKit
Here's some issues I found while reading the code; these may or may not
have security relevance. The misleading error messages are just going to
be annoying for users.
- StringBuffer::reserve() doesn't appear to handle irresponsible length
increases -- it moves the security boundary out to all callers of this
routine, including StringBuffer::write(const char* str, size_t length)
- dtoa() in ./src/utils.h can be made to overflow the 64 byte buffer
provided by _DTOA() if called with DBL_MAX or potentially other inputs.
This is then exposed via StringBuffer operator<<().
- Iconv::convert() in ./src/iconv_utils.cpp looks vulnerable to an integer
overflow if given a too-long str parameter
- copy() in ./src/dictionary_generator.cpp has a misleading error message
"permission denied" that may not reflect the actual error.
- genmatrix() in ./src/dictionary_generator.cpp has a misleading error
message "permission denied" that may not reflect the actual error.
- compile() in ./src/dictionary.cpp has a misleading error message
"permission denied" that may not reflect the actual error.
I couldn't actually tell what the code *does* but it appears to do a good
job of checking calls for errors, checking inputs where that makes sense,
etc. As much as I'd love to see this moved to a C++-14 style, there's
something to be said for code that also appears to be pretty static.
(CVE-2007-3231 was fixed in mecab version 0.96. Maybe mecab is *too*
static, the webpage I found suggests no new changes since 2013.)
Hello, I reviewed mecab version 0.996-6 as checked into cosmic. This is
not a full security audit but rather a quick gauge of maintainability.
- One CVE for mecab in our CVE database.
- mecab is a japanese natural language parser
- Build-Depends: debhelper
- Does not daemonize
- Does not do networking
- postinst looks autogenerated
- No initscript
- No systemd unit files
- No dbus services
- No setuid
- mecab-config and mecab binaries in path
- No sudo fragments
- No udev rules
- Some tests run during the build, I didn't investigate their depth
- No cronjobs
- Relatively clean build logs
- No subprocesses spawned
- Memory management is way too manual; I didn't spot any errors but this
code would benefit from a C++14-aware rewrite.
- Logging looked good
- Environment variables HOME and MECABRC used, looked fine
- No privileged operations
- No cryptography
- No networking
- No privileged portions of code
- No temporary files
- No WebKit
- No JavaScript
- No PolicyKit
Here's some issues I found while reading the code; these may or may not
have security relevance. The misleading error messages are just going to
be annoying for users.
- cppcheck reports uninitialized variable: darts.h: 117]: (error) Uninitialized struct member: tmp_node.right
[src/
- StringBuffer: :reserve( ) doesn't appear to handle irresponsible length :write( const char* str, size_t length)
increases -- it moves the security boundary out to all callers of this
routine, including StringBuffer:
- dtoa() in ./src/utils.h can be made to overflow the 64 byte buffer
provided by _DTOA() if called with DBL_MAX or potentially other inputs.
This is then exposed via StringBuffer operator<<().
- Iconv::convert() in ./src/iconv_ utils.cpp looks vulnerable to an integer
overflow if given a too-long str parameter
- copy() in ./src/dictionar y_generator. cpp has a misleading error message
"permission denied" that may not reflect the actual error.
- genmatrix() in ./src/dictionar y_generator. cpp has a misleading error
message "permission denied" that may not reflect the actual error.
- compile() in ./src/dictionar y.cpp has a misleading error message
"permission denied" that may not reflect the actual error.
I couldn't actually tell what the code *does* but it appears to do a good
job of checking calls for errors, checking inputs where that makes sense,
etc. As much as I'd love to see this moved to a C++-14 style, there's
something to be said for code that also appears to be pretty static.
(CVE-2007-3231 was fixed in mecab version 0.96. Maybe mecab is *too*
static, the webpage I found suggests no new changes since 2013.)
Security team ACK for promoting mecab to main.
Thanks