Skip to content

Commit 034964e

Browse files
ejohnstownphilljj
authored andcommitted
internal: document the name-list truncation rule
GetNameListRaw() stops at the first empty element and drops the rest, as OpenSSH's match_list() does. Record that at the function, along with the two things that keep it safe for the library's own lists: the canned defaults carry only the trailing comma AlgoListSz() strips, and a caller's list is screened by CheckAlgoList() in the ssh.c setters. - leave a pointer in regress.c rather than a second copy of the contract
1 parent cbc5f06 commit 034964e

2 files changed

Lines changed: 13 additions & 7 deletions

File tree

‎src/internal.c‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5902,7 +5902,15 @@ int GetStringRef(word32* strSz, const byte** str,
59025902

59035903
/* Name-list to IDs. *idListSz is capacity in, count out, and bounds every
59045904
* store. One ID per name at most, and names are capped at
5905-
* WOLFSSH_MAX_NAMELIST_CNT, so an idList that size never trips the bound. */
5905+
* WOLFSSH_MAX_NAMELIST_CNT, so an idList that size never trips the bound.
5906+
* An empty element ends the list, silently dropping every name past it, as
5907+
* OpenSSH's match_list() does with a peer proposal. That is safe for the
5908+
* library's own lists on two counts: the canned defaults are built one
5909+
* "name," at a time, so they carry nothing but the trailing comma that
5910+
* AlgoListSz() strips, and a caller's list reaches those fields only
5911+
* through the ssh.c setters, which run CheckAlgoList() and reject every
5912+
* other empty element. A list built at runtime through neither route would
5913+
* lose algorithms; truncation is not an error, only a debug log. */
59065914
static int GetNameListRaw(byte* idList, word32* idListSz,
59075915
const byte* nameList, word32 nameListSz)
59085916
{
@@ -5928,8 +5936,7 @@ static int GetNameListRaw(byte* idList, word32* idListSz,
59285936
* length of the list. Find the commas, or end of list, and then decode
59295937
* the values. A name has a non-zero length, RFC 4251 section 5, so an
59305938
* empty element -- what a leading, doubled, or trailing comma leaves
5931-
* behind -- ends the list. Names past it are dropped rather than
5932-
* rejected, which is what OpenSSH's match_list() does with a peer list.
5939+
* behind -- ends the list, per the contract above.
59335940
*/
59345941

59355942
for (i = 0; i <= nameListSz; i++) {

‎tests/regress.c‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16971,10 +16971,9 @@ typedef struct {
1697116971
/* RFC 4251 section 5: a name-list holds zero or more names separated by
1697216972
* commas, and every name has a non-zero length. So an empty element -- what a
1697316973
* leading, doubled, or trailing comma leaves behind -- can't be a name, and
16974-
* the list ends there. Names past it are dropped rather than rejected, which
16975-
* is what OpenSSH's match_list() does with a peer proposal. A local list never
16976-
* arrives here with an empty element: CheckAlgoList() rejects one up front,
16977-
* except for the single trailing comma that AlgoListSz() strips. */
16974+
* the list ends there. Names past it are dropped rather than rejected. See
16975+
* GetNameListRaw() in src/internal.c for the contract and the CheckAlgoList()
16976+
* invariant the library's own lists rely on. */
1697816977
static const EmptyNameCase emptyNameCases[] = {
1697916978
/* Nothing before the empty element, so nothing to negotiate with. */
1698016979
{ "", ID_UNKNOWN, "zero names" },

0 commit comments

Comments
 (0)