src/haproxy.c: unintentional integer overflow suspected by coverity

[chipitsine]

Tool Name and Version

coverity

Code Report

2254                sslmem = mem - global.maxconn * (int64_t)(STREAM_MAX_COST + 2 * global.tune.bufsize);
2255                global.maxsslconn = sslmem / (global.ssl_session_max_cost + global.ssl_handshake_max_cost);
2256                global.maxsslconn = round_2dig(global.maxsslconn);
2257
2258                if (sslmem <= 0 || global.maxsslconn < sides) {
2259                        ha_alert("Cannot compute the automatic maxsslconn because global.maxconn is already too "
2260                                 "high for the global.memmax value (%d MB). The absolute maximum possible value "
2261                                 "without SSL is %d, but %d was found and SSL is in use.\n",
2262                                 global.rlimit_memmax,
2263                                 (int)(mem / (STREAM_MAX_COST + 2 * global.tune.bufsize)),
2264                                 global.maxconn);
2265                        exit(1);
2266                }
2267
2268                if (global.maxsslconn > sides * global.maxconn)
2269                        global.maxsslconn = sides * global.maxconn;
2270
2271                if (global.mode & (MODE_VERBOSE|MODE_DEBUG))
2272                        fprintf(stderr, "Note: setting global.maxsslconn to %d\n", global.maxsslconn);
2273        }
2274#endif
2275        else if (!global.maxconn) {
2276                /* memmax and maxsslconn are known/unused, compute maxconn automatically */
2277                int sides = !!global.ssl_used_frontend + !!global.ssl_used_backend;
2278                int64_t mem = global.rlimit_memmax * 1048576ULL;
2279                int64_t clearmem;
2280                int retried = 0;
2281
2282                if (global.ssl_used_frontend || global.ssl_used_backend)
    	
CID 1469677 (#3 of 3): Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
overflow_before_widen: Potentially overflowing expression global.tune.sslcachesize * 200 with type int (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type int64_t (64 bits, signed).
    	To avoid overflow, cast either global.tune.sslcachesize or 200 to type int64_t.
2283                        mem -= global.tune.sslcachesize * 200; // about 200 bytes per SSL cache entry

Additional Information

No response

Output of haproxy -vv

no

[wtarreau]

This one is a false positive as the total SSL cache cannot be wider than 2 (or 4?) GB anyway. We could change this to 200ULL though.

[chipitsine]

hmm, coverity picked old source (or we passed old source to it).
issue is still there.

let us wait one more day

[chipitsine]

oops, it is another occurence.

@wtarreau , can you please apply similar cleanup around line 2383 ?

2382                if (global.ssl_used_frontend || global.ssl_used_backend)
    	
CID 1469677 (#2 of 3): Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
overflow_before_widen: Potentially overflowing expression global.tune.sslcachesize * 200 with type int (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type int64_t (64 bits, signed).
    	To avoid overflow, cast either global.tune.sslcachesize or 200 to type int64_t.
2383                        mem -= global.tune.sslcachesize * 200; // about 200 bytes per SSL cache entry
2384
2385                mem -= global.maxzlibmem;

[wtarreau]

Ah, crap, and Iooked without noticing another one! I'll do it as well, thank you Ilya!

[wtarreau]

Now done, coverity should be happy now. There were in fact two other places.

[chipitsine]

it was the only remaining issue. 1st June will start as "0 issues remaining".

I' closing this issue (will reopen tomorrow if coverity is still not happy)