0003-rewrite-wcsnrtombs-to-fix-buffer-overflow-and-other-.patch 3.6 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114
  1. From 3ab2a4e02682df1382955071919d8aa3c3ec40d4 Mon Sep 17 00:00:00 2001
  2. From: Rich Felker <dalias@aerifal.cx>
  3. Date: Thu, 19 Nov 2020 17:12:43 -0500
  4. Subject: [PATCH] rewrite wcsnrtombs to fix buffer overflow and other bugs
  5. the original wcsnrtombs implementation, which has been largely
  6. untouched since 0.5.0, attempted to build input-length-limiting
  7. conversion on top of wcsrtombs, which only limits output length. as
  8. best I recall, this choice was made out of a mix of disdain over
  9. having yet another variant function to implement (added in POSIX 2008;
  10. not standard C) and preference not to switch things around and
  11. implement the wcsrtombs in terms of the more general new function,
  12. probably over namespace issues. the strategy employed was to impose
  13. output limits that would ensure the input limit wasn't exceeded, then
  14. finish up the tail character-at-a-time. unfortunately, none of that
  15. worked correctly.
  16. first, the logic in the wcsrtombs loop was wrong in that it could
  17. easily get stuck making no forward progress, by imposing an output
  18. limit too small to convert even one character.
  19. the character-at-a-time loop that followed was even worse. it made no
  20. effort to ensure that the converted multibyte character would fit in
  21. the remaining output space, only that there was a nonzero amount of
  22. output space remaining. it also employed an incorrect interpretation
  23. of wcrtomb's interface contract for converting the null character,
  24. thereby failing to act on end of input, and remaining space accounting
  25. was subject to unsigned wrap-around. together these errors allow
  26. unbounded overflow of the destination buffer, controlled by input
  27. length limit and input wchar_t string contents.
  28. given the extent to which this function was broken, it's plausible
  29. that most applications that would have been rendered exploitable were
  30. sufficiently broken not to be usable in the first place. however, it's
  31. also plausible that common (especially ASCII-only) inputs succeeded in
  32. the wcsrtombs loop, which mostly worked, while leaving the wildly
  33. erroneous code in the second loop exposed to particular non-ASCII
  34. inputs.
  35. CVE-2020-28928 has been assigned for this issue.
  36. Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
  37. ---
  38. src/multibyte/wcsnrtombs.c | 46 ++++++++++++++++----------------------
  39. 1 file changed, 19 insertions(+), 27 deletions(-)
  40. diff --git a/src/multibyte/wcsnrtombs.c b/src/multibyte/wcsnrtombs.c
  41. index 676932b5..95e25e70 100644
  42. --- a/src/multibyte/wcsnrtombs.c
  43. +++ b/src/multibyte/wcsnrtombs.c
  44. @@ -1,41 +1,33 @@
  45. #include <wchar.h>
  46. +#include <limits.h>
  47. +#include <string.h>
  48. size_t wcsnrtombs(char *restrict dst, const wchar_t **restrict wcs, size_t wn, size_t n, mbstate_t *restrict st)
  49. {
  50. - size_t l, cnt=0, n2;
  51. - char *s, buf[256];
  52. const wchar_t *ws = *wcs;
  53. - const wchar_t *tmp_ws;
  54. -
  55. - if (!dst) s = buf, n = sizeof buf;
  56. - else s = dst;
  57. -
  58. - while ( ws && n && ( (n2=wn)>=n || n2>32 ) ) {
  59. - if (n2>=n) n2=n;
  60. - tmp_ws = ws;
  61. - l = wcsrtombs(s, &ws, n2, 0);
  62. - if (!(l+1)) {
  63. - cnt = l;
  64. - n = 0;
  65. + size_t cnt = 0;
  66. + if (!dst) n=0;
  67. + while (ws && wn) {
  68. + char tmp[MB_LEN_MAX];
  69. + size_t l = wcrtomb(n<MB_LEN_MAX ? tmp : dst, *ws, 0);
  70. + if (l==-1) {
  71. + cnt = -1;
  72. break;
  73. }
  74. - if (s != buf) {
  75. - s += l;
  76. + if (dst) {
  77. + if (n<MB_LEN_MAX) {
  78. + if (l>n) break;
  79. + memcpy(dst, tmp, l);
  80. + }
  81. + dst += l;
  82. n -= l;
  83. }
  84. - wn = ws ? wn - (ws - tmp_ws) : 0;
  85. - cnt += l;
  86. - }
  87. - if (ws) while (n && wn) {
  88. - l = wcrtomb(s, *ws, 0);
  89. - if ((l+1)<=1) {
  90. - if (!l) ws = 0;
  91. - else cnt = l;
  92. + if (!*ws) {
  93. + ws = 0;
  94. break;
  95. }
  96. - ws++; wn--;
  97. - /* safe - this loop runs fewer than sizeof(buf) times */
  98. - s+=l; n-=l;
  99. + ws++;
  100. + wn--;
  101. cnt += l;
  102. }
  103. if (dst) *wcs = ws;
  104. --
  105. 2.20.1