Bug 1975500
| Summary: | [RFE] Zero errno before check strtoul() possible error at ssh_analyze_banner() | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 9 | Reporter: | Marco Benatto <mbenatto> |
| Component: | libssh | Assignee: | Norbert Pócs <npocs> |
| Status: | CLOSED ERRATA | QA Contact: | Alexander Sosedkin <asosedki> |
| Severity: | low | Docs Contact: | |
| Priority: | low | ||
| Version: | 9.0 | CC: | jjelen |
| Target Milestone: | beta | Keywords: | FutureFeature, Triaged |
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | libssh-0.10.4-3.el9 | Doc Type: | No Doc Update |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2023-05-09 08:15:49 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory (libssh bug fix and enhancement update), and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2023:2476 |
Description of problem: Currently libssh uses strtoul function to convert ssh version from string to unsigned long int at ssh_analyze_banner(), in case of error this call returns a code via errno variable. The function uses errno return to check for possible errors when converting the string for both major and minor version values, however it doesn't zero errno value. As it's possible previous errors values may be still stored, even on success of strtoul() call, the verification may fail. This can lead to unpredicted behavior as OpenSSH version in the session will remain uninitialized. int ssh_analyze_banner(ssh_session session, int server) 1222 { . . . openssh = strstr(banner, "OpenSSH"); 1271 if (openssh != NULL) { 1272 char *tmp = NULL; 1273 unsigned long int major = 0UL; 1274 unsigned long int minor = 0UL; 1275 1276 /* 1277 * The banner is typical: 1278 * OpenSSH_5.4 1279 * 012345678901234567890 1280 */ 1281 if (strlen(openssh) > 9) { 1282 major = strtoul(openssh + 8, &tmp, 10); 1283 if ((tmp == (openssh + 8)) || 1284 ((errno == ERANGE) && (major == ULONG_MAX)) || <-------- 1285 ((errno != 0) && (major == 0)) || <--------------------- 1286 ((major < 1) || (major > 100))) { 1287 /* invalid major */ 1288 goto done; 1289 } 1290 1291 minor = strtoul(openssh + 10, &tmp, 10); 1292 if ((tmp == (openssh + 10)) || 1293 ((errno == ERANGE) && (major == ULONG_MAX)) || <-------- 1294 ((errno != 0) && (major == 0)) || <--------------------- 1295 (minor > 100)) { 1296 /* invalid minor */ 1297 goto done; 1298 } 1299 1300 session->openssh = SSH_VERSION_INT(((int) major), ((int) minor), 0); 1301 1302 SSH_LOG(SSH_LOG_PROTOCOL, 1303 "We are talking to an OpenSSH client version: %lu.%lu (%x)", 1304 major, minor, session->openssh); 1305 } 1306 } 1307 This is pretty low prio and can be considered a kind of a hardening.