Mercurial > dropbear
diff svr-authpubkey.c @ 1885:5d8dbb6fdab7
Fix SSH_PUBKEYINFO, limit characters, add tests
We fix a bad_bufptr() failure from a previous commit. We now limit
the allowed characters to those that will definitely be safe
in a shell. Some scripts/programs may use arbitrary environment
variables without escaping correctly - that could be a problem
in a restricted environment.
The current allowed set is a-z A-Z 0-9 .,_-+@
This also adds a test for SSH_PUBKEYINFO, by default it only runs
under github actions (or "act -j build").
author | Matt Johnston <matt@ucc.asn.au> |
---|---|
date | Wed, 16 Mar 2022 17:17:23 +0800 |
parents | f54451afc046 |
children | a7b66ea18632 |
line wrap: on
line diff
--- a/svr-authpubkey.c Wed Mar 16 10:43:24 2022 +0800 +++ b/svr-authpubkey.c Wed Mar 16 17:17:23 2022 +0800 @@ -261,6 +261,7 @@ const char* algo, unsigned int algolen, const unsigned char* keyblob, unsigned int keybloblen) { buffer *options_buf = NULL; + char *info_str = NULL; unsigned int pos, len, infopos, infolen; int ret = DROPBEAR_FAILURE; @@ -339,16 +340,36 @@ goto out; } - /* truncate the line at the space after the base64 data */ + /* find the length of base64 data */ pos = line->pos; for (len = 0; line->pos < line->len; len++) { - if (buf_getbyte(line) == ' ') break; - } - /* findout the length of the public key info */ + if (buf_getbyte(line) == ' ') { + break; + } + } + + /* find out the length of the public key info, stop at the first space */ infopos = line->pos; for (infolen = 0; line->pos < line->len; infolen++) { - if (buf_getbyte(line) == ' ') break; + const char c = buf_getbyte(line); + if (c == ' ') { + break; + } + /* We have an allowlist - authorized_keys lines can't be fully trusted, + some shell scripts may do unsafe things with env var values */ + if (!(isalnum(c) || strchr(".,_-+@", c))) { + TRACE(("Not setting SSH_PUBKEYINFO, special characters")) + infolen = 0; + break; + } } + if (infolen > 0) { + info_str = m_malloc(infolen + 1); + buf_setpos(line, infopos); + strncpy(info_str, buf_getptr(line, infolen), infolen); + } + + /* truncate to base64 data length */ buf_setpos(line, pos); buf_setlen(line, line->pos + len); @@ -359,26 +380,24 @@ /* free pubkey_info if it is filled */ if (ses.authstate.pubkey_info) { m_free(ses.authstate.pubkey_info); - ses.authstate.pubkey_info = NULL; } + if (ret == DROPBEAR_SUCCESS) { if (options_buf) { ret = svr_add_pubkey_options(options_buf, line_num, filename); } - /* save the (optional) public key information */ - if (infolen) { - ses.authstate.pubkey_info = m_malloc(infolen + 1); - if (ses.authstate.pubkey_info) { - strncpy(ses.authstate.pubkey_info,(const char *) buf_getptr(line, infopos), infolen); - ses.authstate.pubkey_info[infolen]='\0'; - } - } + /* take the (optional) public key information */ + ses.authstate.pubkey_info = info_str; + info_str = NULL; } out: if (options_buf) { buf_free(options_buf); } + if (info_str) { + m_free(info_str); + } return ret; }