Skip to content

Commit 2a26172

Browse files
committed
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 924d01b commit 2a26172

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
@@ -5840,7 +5840,15 @@ int GetStringRef(word32* strSz, const byte** str,
58405840

58415841
/* Name-list to IDs. *idListSz is capacity in, count out, and bounds every
58425842
* store. One ID per name at most, and names are capped at
5843-
* WOLFSSH_MAX_NAMELIST_CNT, so an idList that size never trips the bound. */
5843+
* WOLFSSH_MAX_NAMELIST_CNT, so an idList that size never trips the bound.
5844+
* An empty element ends the list, silently dropping every name past it, as
5845+
* OpenSSH's match_list() does with a peer proposal. That is safe for the
5846+
* library's own lists on two counts: the canned defaults are built one
5847+
* "name," at a time, so they carry nothing but the trailing comma that
5848+
* AlgoListSz() strips, and a caller's list reaches those fields only
5849+
* through the ssh.c setters, which run CheckAlgoList() and reject every
5850+
* other empty element. A list built at runtime through neither route would
5851+
* lose algorithms; truncation is not an error, only a debug log. */
58445852
static int GetNameListRaw(byte* idList, word32* idListSz,
58455853
const byte* nameList, word32 nameListSz)
58465854
{
@@ -5866,8 +5874,7 @@ static int GetNameListRaw(byte* idList, word32* idListSz,
58665874
* length of the list. Find the commas, or end of list, and then decode
58675875
* the values. A name has a non-zero length, RFC 4251 section 5, so an
58685876
* empty element -- what a leading, doubled, or trailing comma leaves
5869-
* behind -- ends the list. Names past it are dropped rather than
5870-
* rejected, which is what OpenSSH's match_list() does with a peer list.
5877+
* behind -- ends the list, per the contract above.
58715878
*/
58725879

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

‎tests/regress.c‎

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

0 commit comments

Comments
 (0)