buffer: Convert argc to size_t in ssh_buffer_unpack() as well

Commit c306a693f3 ("buffer: Use size_t for argc argument in
ssh_buffer_(un)pack()") mentioned unpack in the commit log, but it only
touches the pack variants. Extend the conversion to unpack.

Pre-initialize the p pointer to avoid possible use before
initialization in case of early argc check failure.

This fixes build failure:

.../libssh-0.8.6/src/buffer.c: In function 'ssh_buffer_unpack_va':
.../libssh-0.8.6/src/buffer.c:1229:16: error: assuming signed overflow does not occur when simplifying conditional to constant [-Werror=strict-overflow]
             if (argc == -1){
		^

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
This commit is contained in:
Baruch Siach
2019-01-22 13:31:07 +02:00
committed by Andreas Schneider
parent 83d86ef6a5
commit 6c7eaa9e12
2 changed files with 15 additions and 14 deletions

View File

@@ -50,11 +50,11 @@ int _ssh_buffer_pack(struct ssh_buffer_struct *buffer,
_ssh_buffer_pack((buffer), (format), __VA_NARG__(__VA_ARGS__), __VA_ARGS__, SSH_BUFFER_PACK_END) _ssh_buffer_pack((buffer), (format), __VA_NARG__(__VA_ARGS__), __VA_ARGS__, SSH_BUFFER_PACK_END)
int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer, int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer,
const char *format, int argc, const char *format, size_t argc,
va_list ap); va_list ap);
int _ssh_buffer_unpack(struct ssh_buffer_struct *buffer, int _ssh_buffer_unpack(struct ssh_buffer_struct *buffer,
const char *format, const char *format,
int argc, size_t argc,
...); ...);
#define ssh_buffer_unpack(buffer, format, ...) \ #define ssh_buffer_unpack(buffer, format, ...) \
_ssh_buffer_unpack((buffer), (format), __VA_NARG__(__VA_ARGS__), __VA_ARGS__, SSH_BUFFER_PACK_END) _ssh_buffer_unpack((buffer), (format), __VA_NARG__(__VA_ARGS__), __VA_ARGS__, SSH_BUFFER_PACK_END)

View File

@@ -1082,11 +1082,11 @@ int _ssh_buffer_pack(struct ssh_buffer_struct *buffer,
*/ */
int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer, int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer,
const char *format, const char *format,
int argc, size_t argc,
va_list ap) va_list ap)
{ {
int rc = SSH_ERROR; int rc = SSH_ERROR;
const char *p, *last; const char *p = format, *last;
union { union {
uint8_t *byte; uint8_t *byte;
uint16_t *word; uint16_t *word;
@@ -1100,16 +1100,21 @@ int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer,
size_t len, rlen, max_len; size_t len, rlen, max_len;
ssh_string tmp_string = NULL; ssh_string tmp_string = NULL;
va_list ap_copy; va_list ap_copy;
int count; /* int for size comparison with argc */ size_t count;
max_len = ssh_buffer_get_len(buffer); max_len = ssh_buffer_get_len(buffer);
/* copy the argument list in case a rollback is needed */ /* copy the argument list in case a rollback is needed */
va_copy(ap_copy, ap); va_copy(ap_copy, ap);
for (p = format, count = 0; *p != '\0'; p++, count++) { if (argc > 256) {
rc = SSH_ERROR;
goto cleanup;
}
for (count = 0; *p != '\0'; p++, count++) {
/* Invalid number of arguments passed */ /* Invalid number of arguments passed */
if (argc != -1 && count > argc) { if (count > argc) {
rc = SSH_ERROR; rc = SSH_ERROR;
goto cleanup; goto cleanup;
} }
@@ -1232,7 +1237,7 @@ int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer,
} }
} }
if (argc != -1 && argc != count) { if (argc != count) {
rc = SSH_ERROR; rc = SSH_ERROR;
} }
@@ -1241,11 +1246,7 @@ cleanup:
/* Check if our canary is intact, if not something really bad happened */ /* Check if our canary is intact, if not something really bad happened */
uint32_t canary = va_arg(ap, uint32_t); uint32_t canary = va_arg(ap, uint32_t);
if (canary != SSH_BUFFER_PACK_END){ if (canary != SSH_BUFFER_PACK_END){
if (argc == -1){ abort();
rc = SSH_ERROR;
} else {
abort();
}
} }
} }
@@ -1340,7 +1341,7 @@ cleanup:
*/ */
int _ssh_buffer_unpack(struct ssh_buffer_struct *buffer, int _ssh_buffer_unpack(struct ssh_buffer_struct *buffer,
const char *format, const char *format,
int argc, size_t argc,
...) ...)
{ {
va_list ap; va_list ap;