haproxy icon indicating copy to clipboard operation
haproxy copied to clipboard

Static analyze warning in quic_conn.c

Open OlegGavri opened this issue 1 year ago • 2 comments

Tool Name and Version

svacer

Code Report

I use haproxy 2.6 and made static analyze of source code. svacer analyze tool found warning in quic_conn.c in function qc_try_rm_hp. This warning present in old versions before 3097be92f184457433c8e3fa03b89381ae7dc1c6 commit on lines 6094, 6095. quic_packet_type_enc_level can return -1, and variable tel can be used as index of array qel. It can lead to buffer overflow.
Can we check this warning, write why I can ignore it. Or may be fix it in haproxy26 if it is real problem?
Thank you.

Additional Information

No response

Output of haproxy -vv

HAProxy version 2023-10-31-dev-3quic 2023/12/02 - https://haproxy.org/
Status: long-term supported branch - will stop receiving fixes around Q2 2027.
Known bugs: https://github.com/haproxy/haproxy/issues?q=is:issue+is:open
Running on: Linux 4.FORCE01 #0 SMP Sat Dec 2 09:12:26 2023 x86_64
Build options :
  TARGET  = linux-glibc
  CPU     = generic
  CC      = x86_64-openwrt-linux-gnu-gcc
  CFLAGS  = -Os -pipe -fno-caller-saves -fno-plt -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -ffile-prefix-map=/home/jenkins/workspace/owrtmain_dev/confident_add/haproxy-2023-10-31-dev=haproxy-2023-10-31-dev -Wformat -Werror=format-security -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro /home/jenkins/workspace/owrtmain_dev/staging_dir/target-x86_64_glibc/usr/include/openssl3 -fno-strict-aliasing -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter -Wno-clobbered -Wno-missing-field-initializers -Wno-cast-function-type -Wtype-limits -Wshift-negative-value -Wshift-overflow=2 -Wduplicated-cond -Wnull-dereference -fwrapv -fasynchronous-unwind-tables -Wno-null-dereference -DBUFSIZE=32768 -DMAXREWRITE=8192 -DSYSTEM_MAXCONN=165530 -DMAX_SESS_STKCTR=20
  OPTIONS = USE_PCRE=1 USE_PCRE_JIT=1 USE_LINUX_TPROXY=1 USE_LINUX_SPLICE=1 USE_OPENSSL=1 USE_ZLIB=1 USE_TFO=1 USE_QUIC=1 USE_PROMEX=1
  DEBUG   = -DDEBUG_STRICT -DDEBUG_MEMORY_POOLS

Feature list : -51DEGREES +ACCEPT4 +BACKTRACE -CLOSEFROM +CPU_AFFINITY +CRYPT_H -DEVICEATLAS +DL -ENGINE +EPOLL -EVPORTS +GETADDRINFO -KQUEUE +LIBCRYPT +LINUX_SPLICE +LINUX_TPROXY -LUA -MEMORY_PROFILING +NETFILTER +NS -OBSOLETE_LINKER +OPENSSL -OT +PCRE -PCRE2 -PCRE2_JIT +PCRE_JIT +POLL +PRCTL -PROCCTL +PROMEX +QUIC +RT -SLZ -STATIC_PCRE -STATIC_PCRE2 -SYSTEMD +TFO +THREAD +THREAD_DUMP +TPROXY -WURFL +ZLIB

Default settings :
  bufsize = 32768, maxrewrite = 8192, maxpollevents = 200

Built with multi-threading support (MAX_THREADS=64, default=2).
Built with OpenSSL version : OpenSSL 3.0.7+quic 1 Nov 2022
Running on OpenSSL version : OpenSSL 3.0.7+quic 1 Nov 2022
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
OpenSSL providers loaded : default
Built with the Prometheus exporter as a service
Built with network namespace support.
Support for malloc_trim() is enabled.
Built with zlib version : 1.2.12
Running on zlib version : 1.2.12
Compression algorithms supported : identity("identity"), deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT IP_FREEBIND
Built with PCRE version : 8.45 2021-06-15
Running on PCRE version : 8.45 2021-06-15
PCRE library supports JIT : yes
Encrypted password support via crypt(3): yes
Built with gcc compiler version 8.3.0

Available polling systems :
      epoll : pref=300,  test result OK
       poll : pref=200,  test result OK
     select : pref=150,  test result OK
Total: 3 (3 usable), will use epoll.

Available multiplexer protocols :
(protocols marked as <default> cannot be specified using 'proto' keyword)
       quic : mode=HTTP  side=FE     mux=QUIC  flags=HTX|NO_UPG|FRAMED
         h2 : mode=HTTP  side=FE|BE  mux=H2    flags=HTX|HOL_RISK|NO_UPG
       fcgi : mode=HTTP  side=BE     mux=FCGI  flags=HTX|HOL_RISK|NO_UPG
  <default> : mode=HTTP  side=FE|BE  mux=H1    flags=HTX
         h1 : mode=HTTP  side=FE|BE  mux=H1    flags=HTX|NO_UPG
  <default> : mode=TCP   side=FE|BE  mux=PASS  flags=
       none : mode=TCP   side=FE|BE  mux=PASS  flags=NO_UPG

Available services : prometheus-exporter
Available filters :
	[CACHE] cache
	[CLIENT_ID] client_id
	[COMP] compression
	[CSRF] csrf
	[FCGI] fcgi-app
	[SPOE] spoe
	[TRACE] trace

OlegGavri avatar Dec 13 '23 17:12 OlegGavri

coverity is already used as static analyzer by the CI. I am not sure we want to use another tool which would detect the same issues and also the false positive issue. You can have a look at the issues reported by coverity+CI with a simple search as follows: https://github.com/haproxy/haproxy/issues?q=is%3Aclosed+CID

haproxyFred avatar Dec 14 '23 10:12 haproxyFred

@OlegGavri , is it compliance requirement ? I've found that svacer is affiliated with Russian governmental IT activities https://svacer.ispras.ru/mediawiki/index.php?title=Svacer

is there a requirement from some governmental org to pass svacer checks ?

also, I checked reports on coverity itself. there were 2 of them, however, details were deleted by coverity (but I can try to unapprove them to see whether coverity will provide fresh report)

image

chipitsine avatar Dec 14 '23 11:12 chipitsine