shadow-maint / shadow

Upstream shadow tree
Other
287 stars 227 forks source link

Fix lib/port.c issues #1037

Closed alejandro-colomar closed 6 days ago

alejandro-colomar commented 1 week ago

Closes: https://github.com/shadow-maint/shadow/issues/1036


Revisions:

v1b - tfix ``` $ git range-diff string gh/port port 1: 9c18ae98 = 1: 9c18ae98 lib/port.c: getttyuser(): Remove dead code 2: eee9766c = 2: eee9766c lib/port.c: getttyuser(): Use goto to break out of nested loops 3: 7494e4bc = 3: 7494e4bc lib/port.c: getportent(): Rename goto label 4: 4d43f0bf = 4: 4d43f0bf lib/port.c: getportent(): Remove obvious comments 5: c47a94e5 ! 5: 08bbebb3 lib/port.c: getportent(): Make sure there are at least 2 ':' in the line @@ lib/port.c: next: * TTY devices. */ -+ if (strchr(cp, ":") == NULL) ++ if (strchr(cp, ':') == NULL) + goto next; + port.pt_names = ttys; @@ lib/port.c: next: * The last entry in the list is a NULL pointer. */ -+ if (strchr(cp, ":") == NULL) ++ if (strchr(cp, ':') == NULL) + goto next; + if (':' != *cp) { 6: 730b12e7 = 6: 5f76b80a lib/port.c: getportent(): Make sure the aren't too many fields in the CSV 7: d89b96c0 ! 7: c85f44de lib/port.c: getportent(): Use equivalent code to parse equally-formatted fields @@ lib/port.c: next: /* * Get the list of user names. It is the second colon @@ lib/port.c: next: - if (strchr(cp, ":") == NULL) + if (strchr(cp, ':') == NULL) goto next; - if (':' != *cp) { 8: 424423b7 = 8: 156d21b5 lib/port.c: getttyuser(): Use pointer arithmetic to simplify ```
v1c - Rebase ``` $ git range-diff 9c18ae98^..gh/port string..port 1: 9c18ae98 = 1: 259fa993 lib/port.c: getttyuser(): Remove dead code 2: eee9766c = 2: 7cf7edb6 lib/port.c: getttyuser(): Use goto to break out of nested loops 3: 7494e4bc = 3: e2658dd4 lib/port.c: getportent(): Rename goto label 4: 4d43f0bf ! 4: d40831e8 lib/port.c: getportent(): Remove obvious comments @@ lib/port.c: static struct port *getportent (void) goto next; - } + -+ *strchrnul(buf, '\n') = '\0'; ++ stpcpy(strchrnul(buf, '\n'), ""); /* * Get the name of the TTY device. It is the first colon @@ lib/port.c: next: * TTY devices. */ -- *strchrnul(buf, '\n') = '\0'; +- stpcpy(strchrnul(buf, '\n'), ""); - port.pt_names = ttys; for (cp = buf, j = 0; j < PORT_TTY; j++) { 5: 08bbebb3 = 5: 0e695174 lib/port.c: getportent(): Make sure there are at least 2 ':' in the line 6: 5f76b80a = 6: 8c24a030 lib/port.c: getportent(): Make sure the aren't too many fields in the CSV 7: c85f44de = 7: 6cc67d2c lib/port.c: getportent(): Use equivalent code to parse equally-formatted fields 8: 156d21b5 = 8: 24ed30e1 lib/port.c: getttyuser(): Use pointer arithmetic to simplify ```
v1d - Rebase ``` $ git range-diff gh/string..gh/port string..port 1: 259fa993 = 1: 8777e91e lib/port.c: getttyuser(): Remove dead code 2: 7cf7edb6 = 2: c071c9ca lib/port.c: getttyuser(): Use goto to break out of nested loops 3: e2658dd4 = 3: f043203f lib/port.c: getportent(): Rename goto label 4: d40831e8 = 4: 265a42ad lib/port.c: getportent(): Remove obvious comments 5: 0e695174 = 5: f5e4be59 lib/port.c: getportent(): Make sure there are at least 2 ':' in the line 6: 8c24a030 = 6: 2fd4d747 lib/port.c: getportent(): Make sure the aren't too many fields in the CSV 7: 6cc67d2c = 7: 76023010 lib/port.c: getportent(): Use equivalent code to parse equally-formatted fields 8: 24ed30e1 = 8: 7446c282 lib/port.c: getttyuser(): Use pointer arithmetic to simplify ```
v2 - Fix typo. - Use strsep(3) to simplify ``` $ git range-diff string gh/port port 1: 8777e91e = 1: 8777e91e lib/port.c: getttyuser(): Remove dead code 2: c071c9ca = 2: c071c9ca lib/port.c: getttyuser(): Use goto to break out of nested loops 3: f043203f = 3: f043203f lib/port.c: getportent(): Rename goto label 4: 265a42ad = 4: 265a42ad lib/port.c: getportent(): Remove obvious comments 5: f5e4be59 ! 5: 61fe7a00 lib/port.c: getportent(): Make sure there are at least 2 ':' in the line @@ lib/port.c: next: * TTY devices. */ -+ if (strchr(cp, ':') == NULL) ++ if (strchr(buf, ':') == NULL) + goto next; + port.pt_names = ttys; 6: 2fd4d747 = 6: 92ab5c47 lib/port.c: getportent(): Make sure the aren't too many fields in the CSV 7: 76023010 = 7: 2358cba9 lib/port.c: getportent(): Use equivalent code to parse equally-formatted fields 8: 7446c282 = 8: 4e18ed4c lib/port.c: getttyuser(): Use pointer arithmetic to simplify -: -------- > 9: ed43e774 lib/port.c: getportent(): Align variables -: -------- > 10: f38e7d58 lib/port.c: getportent(): Use strsep(3) instead of its pattern ```
v2b - Rebase ``` $ git range-diff gh/string..gh/port string..port 1: 8777e91e = 1: 4272a71d lib/port.c: getttyuser(): Remove dead code 2: c071c9ca = 2: af0d7aa3 lib/port.c: getttyuser(): Use goto to break out of nested loops 3: f043203f = 3: d955ed89 lib/port.c: getportent(): Rename goto label 4: 265a42ad = 4: 8a583189 lib/port.c: getportent(): Remove obvious comments 5: 61fe7a00 = 5: a49c9193 lib/port.c: getportent(): Make sure there are at least 2 ':' in the line 6: 92ab5c47 = 6: c9d0ba17 lib/port.c: getportent(): Make sure the aren't too many fields in the CSV 7: 2358cba9 = 7: 602b6363 lib/port.c: getportent(): Use equivalent code to parse equally-formatted fields 8: 4e18ed4c = 8: 287722d5 lib/port.c: getttyuser(): Use pointer arithmetic to simplify 9: ed43e774 = 9: bdb44469 lib/port.c: getportent(): Align variables 10: f38e7d58 = 10: b555c046 lib/port.c: getportent(): Use strsep(3) instead of its pattern ```
v2c - Rebase ``` $ git range-diff 4272a71d^..gh/port string..port 1: 4272a71d = 1: 77221a0b lib/port.c: getttyuser(): Remove dead code 2: af0d7aa3 = 2: a8d4a0be lib/port.c: getttyuser(): Use goto to break out of nested loops 3: d955ed89 = 3: 3fc399e9 lib/port.c: getportent(): Rename goto label 4: 8a583189 = 4: 7b88f533 lib/port.c: getportent(): Remove obvious comments 5: a49c9193 = 5: ce972d67 lib/port.c: getportent(): Make sure there are at least 2 ':' in the line 6: c9d0ba17 = 6: ae60c105 lib/port.c: getportent(): Make sure the aren't too many fields in the CSV 7: 602b6363 = 7: 75d52d74 lib/port.c: getportent(): Use equivalent code to parse equally-formatted fields 8: 287722d5 = 8: 28142c1c lib/port.c: getttyuser(): Use pointer arithmetic to simplify 9: bdb44469 = 9: 8e0daccd lib/port.c: getportent(): Align variables 10: b555c046 = 10: a7ba79e4 lib/port.c: getportent(): Use strsep(3) instead of its pattern ```
v2d - Rebase ``` $ git range-diff 77221a0b^..gh/port shadow/master..port 1: 77221a0b = 1: 04229ae8 lib/port.c: getttyuser(): Remove dead code 2: a8d4a0be = 2: 08d9c883 lib/port.c: getttyuser(): Use goto to break out of nested loops 3: 3fc399e9 = 3: 34386a8f lib/port.c: getportent(): Rename goto label 4: 7b88f533 = 4: c1800b35 lib/port.c: getportent(): Remove obvious comments 5: ce972d67 = 5: 6d509bc0 lib/port.c: getportent(): Make sure there are at least 2 ':' in the line 6: ae60c105 = 6: b72e4440 lib/port.c: getportent(): Make sure the aren't too many fields in the CSV 7: 75d52d74 = 7: 7cdf4542 lib/port.c: getportent(): Use equivalent code to parse equally-formatted fields 8: 28142c1c = 8: 57a7d3a0 lib/port.c: getttyuser(): Use pointer arithmetic to simplify 9: 8e0daccd = 9: dc935c48 lib/port.c: getportent(): Align variables 10: a7ba79e4 = 10: a9045016 lib/port.c: getportent(): Use strsep(3) instead of its pattern ```
v2e - Detail commit message ``` $ git range-diff master gh/port port 1: 04229ae8 = 1: 04229ae8 lib/port.c: getttyuser(): Remove dead code 2: 08d9c883 = 2: 08d9c883 lib/port.c: getttyuser(): Use goto to break out of nested loops 3: 34386a8f = 3: 34386a8f lib/port.c: getportent(): Rename goto label 4: c1800b35 = 4: c1800b35 lib/port.c: getportent(): Remove obvious comments 5: 6d509bc0 = 5: 6d509bc0 lib/port.c: getportent(): Make sure there are at least 2 ':' in the line 6: b72e4440 ! 6: 9eb6c496 lib/port.c: getportent(): Make sure the aren't too many fields in the CSV @@ Commit message Otherwise, the line is invalidly formatted, and we ignore it. + Detailed explanation: + + There are two conditions on which we break out of the loops that precede + these added checks: + + - j is too big (we've exhausted the space in the static arrays) + + $ grep -r -e PORT_TTY -e PORT_IDS lib/port.* + lib/port.c: static char *ttys[PORT_TTY + 1]; /* some pointers to tty names */ + lib/port.c: static char *users[PORT_IDS + 1]; /* some pointers to user ids */ + lib/port.c: for (cp = buf, j = 0; j < PORT_TTY; j++) { + lib/port.c: if ((',' == *cp) && (j < PORT_IDS)) { + lib/port.h: * PORT_IDS - Allowable number of IDs per entry. + lib/port.h: * PORT_TTY - Allowable number of TTYs per entry. + lib/port.h:#define PORT_IDS 64 + lib/port.h:#define PORT_TTY 64 + + - strpbrk(3) found a ':', which signals the end of the outer + colon-separated list. + + If the first character in the remainder of the string is not a ':', it + means we've exhausted the array size, but the CSV string was longer, so + we'd be truncating it. Consider the entire line invalid, and skip it. + Signed-off-by: Alejandro Colomar ## lib/port.c ## 7: 7cdf4542 = 7: c65299f1 lib/port.c: getportent(): Use equivalent code to parse equally-formatted fields 8: 57a7d3a0 = 8: 058beb58 lib/port.c: getttyuser(): Use pointer arithmetic to simplify 9: dc935c48 = 9: a9582a8a lib/port.c: getportent(): Align variables 10: a9045016 = 10: 9f88eb69 lib/port.c: getportent(): Use strsep(3) instead of its pattern ```
v2f - wfix ``` $ git range-diff master gh/port port 1: 04229ae8 = 1: 04229ae8 lib/port.c: getttyuser(): Remove dead code 2: 08d9c883 = 2: 08d9c883 lib/port.c: getttyuser(): Use goto to break out of nested loops 3: 34386a8f = 3: 34386a8f lib/port.c: getportent(): Rename goto label 4: c1800b35 = 4: c1800b35 lib/port.c: getportent(): Remove obvious comments 5: 6d509bc0 = 5: 6d509bc0 lib/port.c: getportent(): Make sure there are at least 2 ':' in the line 6: 9eb6c496 ! 6: 53602f8b lib/port.c: getportent(): Make sure the aren't too many fields in the CSV @@ Commit message lib/port.h:#define PORT_IDS 64 lib/port.h:#define PORT_TTY 64 - - strpbrk(3) found a ':', which signals the end of the outer - colon-separated list. + - strpbrk(3) found a ':', which signals the end of the comma-sepatated + list, and the start of the next colon-separated field. If the first character in the remainder of the string is not a ':', it - means we've exhausted the array size, but the CSV string was longer, so + means we've exhausted the array size, but the CSV list was longer, so we'd be truncating it. Consider the entire line invalid, and skip it. Signed-off-by: Alejandro Colomar 7: c65299f1 = 7: d6073f28 lib/port.c: getportent(): Use equivalent code to parse equally-formatted fields 8: 058beb58 = 8: fa2b3ac2 lib/port.c: getttyuser(): Use pointer arithmetic to simplify 9: a9582a8a = 9: fe878354 lib/port.c: getportent(): Align variables 10: 9f88eb69 = 10: 7d721a07 lib/port.c: getportent(): Use strsep(3) instead of its pattern ```
alejandro-colomar commented 1 week ago

Queued after https://github.com/shadow-maint/shadow/pull/1035.