I reviewed sane-airscan 0.99.24-1 as checked into hirsute. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.
sane-airscan is a package that provides a sane backend which supports
driverless network-connected scanners.
- CVE History:
- No CVE history, this is a relatively new package that was only first
released in December 2019 and only exists in Ubuntu since groovy.
- Build-Depends
- gnutls, libavahi, libxml2 and libjpeg-turbo are all depedencies
- No pre/post inst/rm scripts
- No init scripts
- No systemd units
- No dbus services
- No setuid binaries
- 1 binary in PATH
- -rwxr-xr-x root/root 175312 2021-02-08 01:49 ./usr/bin/airscan-discover
- No sudo fragments
- No polkit files
- No udev rules
- unit tests / autopkgtests
- unit tests are compiled in the build log but do not appear to be run
during the build - these appear to just do simple testing of image
decoding, URI parsing and HTTP multipart decoding. There are also tests
for the zeroconf handling and these appear to be more extensive. These
should be executed during the build itself.
- there are no autopkgtests
- No cron jobs
- Build logs are quite clean
- No processes spawned
- Memory management
- sane-airscan is all in C so there is a fair amount of dynamic memory
management - this all appears to be done quite carefully, Coverity only
found a couple issues (detailed below) but in general this is quite
good quality code.
- File IO
- Loads inifile from /etc/sane.d or from path specified in environment
variable CONFIG_PATH_ENV - this appears to be quite defensive and safe
- Extensive logging but this all appears safe
- Environment variable usage:
- CONFIG_PATH_ENV - as noted above for the path to the configuration directory
- HOME - use of this is safely, checking length etc before use - this is
used to be able to expand ~ in configuration settings to $HOME etc
- CONFIG_ENV_AIRSCAN_DEBUG - this is used to be able to turn on debug
logging
- No use of privileged functions
- Use of cryptography / random number sources
- Uses /dev/urandom to generate random value for retransmit timer - this
is not security sensitive but /dev/urandom is a good choice since this
is non-blocking
- Uses gnutls for SHA256 hash and TLS for HTTPS - but doesn't perform any
certificate verification of the remote host - but this is expected
since scanners on a local network are not going to have FQDN nor
appropriately issued certificates (ie they are likely to just be
self-signed)
- No use of temp files
- Use of networking is generally defensive but relies on dependent
libraries to do parsing of image formats etc so this risk is delegated
- No use of WebKit
- No use of PolicyKit
- No significant cppcheck results
- A couple significant Coverity results which should be relatively easy to
fix - I have reported these upstream at https://github.com/alexpevzner/sane-airscan/issues/131 but the two most
important ones are:
airscan-http.c:1261:
1. var_decl: Declaring variable "parser" without initializer.
airscan-http.c:1265:
2. path: Condition "skip_line", taking true branch.
airscan-http.c:1267:
3. path: Condition "s", taking true branch.
airscan-http.c:1275:
4. uninit_use_in_call: Using uninitialized value "parser.data" when calling "http_parser_init".
http_parser.c:2220:
4.1. read_parm_fld: Reading a parameter field.
------
airscan-os.c:109
Type: String not null terminated (STRING_NULL)
airscan-os.c:79:
1. string_null_argument: Function "readlink" does not terminate string "*os_progname_buf". [Note: The source code implementation of the function has been overridden by a builtin model.]
airscan-os.c:80:
2. path: Condition "rc < 0", taking false branch.
airscan-os.c:109:
3. string_null: Passing unterminated string "os_progname_buf" to "strrchr", which expects a null-terminated string.
------
- No significant shellcheck results
- No significant bandit results
I noticed licensecheck couldn't find a license on most of the source file -
should this be explicitly listed in each?
Security team ACK for promoting sane-airscan to main conditional on the
unit tests being plumbed into the build or autopkgtests added which at
least run these unit tests instead if they can't easily be run at
compile-time.
I reviewed sane-airscan 0.99.24-1 as checked into hirsute. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.
sane-airscan is a package that provides a sane backend which supports
driverless network-connected scanners.
- CVE History: airscan- discover
- No CVE history, this is a relatively new package that was only first
released in December 2019 and only exists in Ubuntu since groovy.
- Build-Depends
- gnutls, libavahi, libxml2 and libjpeg-turbo are all depedencies
- No pre/post inst/rm scripts
- No init scripts
- No systemd units
- No dbus services
- No setuid binaries
- 1 binary in PATH
- -rwxr-xr-x root/root 175312 2021-02-08 01:49 ./usr/bin/
- No sudo fragments
- No polkit files
- No udev rules
- unit tests / autopkgtests
- unit tests are compiled in the build log but do not appear to be run
during the build - these appear to just do simple testing of image
decoding, URI parsing and HTTP multipart decoding. There are also tests
for the zeroconf handling and these appear to be more extensive. These
should be executed during the build itself.
- there are no autopkgtests
- No cron jobs
- Build logs are quite clean
- No processes spawned ENV_AIRSCAN_ DEBUG - this is used to be able to turn on debug
- Memory management
- sane-airscan is all in C so there is a fair amount of dynamic memory
management - this all appears to be done quite carefully, Coverity only
found a couple issues (detailed below) but in general this is quite
good quality code.
- File IO
- Loads inifile from /etc/sane.d or from path specified in environment
variable CONFIG_PATH_ENV - this appears to be quite defensive and safe
- Extensive logging but this all appears safe
- Environment variable usage:
- CONFIG_PATH_ENV - as noted above for the path to the configuration directory
- HOME - use of this is safely, checking length etc before use - this is
used to be able to expand ~ in configuration settings to $HOME etc
- CONFIG_
logging
- No use of privileged functions
- Use of cryptography / random number sources
- Uses /dev/urandom to generate random value for retransmit timer - this
is not security sensitive but /dev/urandom is a good choice since this
is non-blocking
- Uses gnutls for SHA256 hash and TLS for HTTPS - but doesn't perform any
certificate verification of the remote host - but this is expected
since scanners on a local network are not going to have FQDN nor
appropriately issued certificates (ie they are likely to just be
self-signed)
- No use of temp files
- Use of networking is generally defensive but relies on dependent
libraries to do parsing of image formats etc so this risk is delegated
- No use of WebKit
- No use of PolicyKit
- No significant cppcheck results /github. com/alexpevzner /sane-airscan/ issues/ 131 but the two most
- A couple significant Coverity results which should be relatively easy to
fix - I have reported these upstream at
https:/
important ones are:
-----
airscan-http.c:1275
Type: Uninitialized pointer read (UNINIT)
airscan- http.c: 1261: http.c: 1265: http.c: 1267: http.c: 1275:
1. var_decl: Declaring variable "parser" without initializer.
airscan-
2. path: Condition "skip_line", taking true branch.
airscan-
3. path: Condition "s", taking true branch.
airscan-
4. uninit_use_in_call: Using uninitialized value "parser.data" when calling "http_parser_init".
http_parser.c:2220:
4.1. read_parm_fld: Reading a parameter field.
------
airscan-os.c:109
Type: String not null terminated (STRING_NULL)
airscan-os.c:79: null_argument: Function "readlink" does not terminate string "*os_progname_buf". [Note: The source code implementation of the function has been overridden by a builtin model.]
1. string_
airscan-os.c:80:
2. path: Condition "rc < 0", taking false branch.
airscan-os.c:109:
3. string_null: Passing unterminated string "os_progname_buf" to "strrchr", which expects a null-terminated string.
------
- No significant shellcheck results
- No significant bandit results
I noticed licensecheck couldn't find a license on most of the source file -
should this be explicitly listed in each?
Security team ACK for promoting sane-airscan to main conditional on the
unit tests being plumbed into the build or autopkgtests added which at
least run these unit tests instead if they can't easily be run at
compile-time.