Fix an issue with the strcoll CVE patch

Expanded types for some variables to prevent overflow.
This commit is contained in:
Siddhesh Poyarekar 2013-08-22 11:10:25 +05:30
parent e3b637cf01
commit 735547c10e
2 changed files with 27 additions and 26 deletions

View File

@ -1,5 +1,5 @@
diff --git a/string/strcoll_l.c b/string/strcoll_l.c diff --git a/string/strcoll_l.c b/string/strcoll_l.c
index ecda08f..ec630fe 100644 index ecda08f..bb34a72 100644
--- a/string/strcoll_l.c --- a/string/strcoll_l.c
+++ b/string/strcoll_l.c +++ b/string/strcoll_l.c
@@ -41,11 +41,434 @@ @@ -41,11 +41,434 @@
@ -10,15 +10,15 @@ index ecda08f..ec630fe 100644
+typedef struct +typedef struct
+{ +{
+ int len; /* Length of the current sequence. */ + int len; /* Length of the current sequence. */
+ int val; /* Position of the sequence relative to the + size_t val; /* Position of the sequence relative to the
+ previous non-ignored sequence. */ + previous non-ignored sequence. */
+ size_t idxnow; /* Current index in sequences. */ + size_t idxnow; /* Current index in sequences. */
+ size_t idxmax; /* Maximum index in sequences. */ + size_t idxmax; /* Maximum index in sequences. */
+ size_t idxcnt; /* Current count of indeces. */ + size_t idxcnt; /* Current count of indices. */
+ size_t backw; /* Current Backward sequence index. */ + size_t backw; /* Current Backward sequence index. */
+ size_t backw_stop; /* Index where the backward sequences stop. */ + size_t backw_stop; /* Index where the backward sequences stop. */
+ const USTRING_TYPE *us; /* The string. */ + const USTRING_TYPE *us; /* The string. */
+ int32_t *idxarr; /* Array to cache weight indeces. */ + int32_t *idxarr; /* Array to cache weight indices. */
+ unsigned char *rulearr; /* Array to cache rules. */ + unsigned char *rulearr; /* Array to cache rules. */
+ unsigned char rule; /* Saved rule for the first sequence. */ + unsigned char rule; /* Saved rule for the first sequence. */
+ int32_t idx; /* Index to weight of the current sequence. */ + int32_t idx; /* Index to weight of the current sequence. */
@ -28,14 +28,14 @@ index ecda08f..ec630fe 100644
+ const USTRING_TYPE *back_us; /* Beginning of the backward sequence. */ + const USTRING_TYPE *back_us; /* Beginning of the backward sequence. */
+} coll_seq; +} coll_seq;
+ +
+/* Get next sequence. The weight indeces are cached, so we don't need to +/* Get next sequence. The weight indices are cached, so we don't need to
+ traverse the string. */ + traverse the string. */
+static void +static void
+get_next_seq_cached (coll_seq *seq, int nrules, int pass, +get_next_seq_cached (coll_seq *seq, int nrules, int pass,
+ const unsigned char *rulesets, + const unsigned char *rulesets,
+ const USTRING_TYPE *weights) + const USTRING_TYPE *weights)
+{ +{
+ int val = seq->val = 0; + size_t val = seq->val = 0;
+ int len = seq->len; + int len = seq->len;
+ size_t backw_stop = seq->backw_stop; + size_t backw_stop = seq->backw_stop;
+ size_t backw = seq->backw; + size_t backw = seq->backw;
@ -50,7 +50,7 @@ index ecda08f..ec630fe 100644
+ ++val; + ++val;
+ if (backw_stop != ~0ul) + if (backw_stop != ~0ul)
+ { + {
+ /* The is something pushed. */ + /* There is something pushed. */
+ if (backw == backw_stop) + if (backw == backw_stop)
+ { + {
+ /* The last pushed character was handled. Continue + /* The last pushed character was handled. Continue
@ -62,7 +62,7 @@ index ecda08f..ec630fe 100644
+ } + }
+ else + else
+ { + {
+ /* Nothing anymore. The backward sequence + /* Nothing any more. The backward sequence
+ ended with the last sequence in the string. */ + ended with the last sequence in the string. */
+ idxnow = ~0ul; + idxnow = ~0ul;
+ break; + break;
@ -88,7 +88,7 @@ index ecda08f..ec630fe 100644
+ { + {
+ /* No sequence at all or just one. */ + /* No sequence at all or just one. */
+ if (idxcnt == idxmax) + if (idxcnt == idxmax)
+ /* Note that seq1len is still zero. */ + /* Note that LEN is still zero. */
+ break; + break;
+ +
+ backw_stop = ~0ul; + backw_stop = ~0ul;
@ -117,7 +117,7 @@ index ecda08f..ec630fe 100644
+ const USTRING_TYPE *extra, const int32_t *indirect) + const USTRING_TYPE *extra, const int32_t *indirect)
+{ +{
+#include WEIGHT_H +#include WEIGHT_H
+ int val = seq->val = 0; + size_t val = seq->val = 0;
+ int len = seq->len; + int len = seq->len;
+ size_t backw_stop = seq->backw_stop; + size_t backw_stop = seq->backw_stop;
+ size_t backw = seq->backw; + size_t backw = seq->backw;
@ -133,7 +133,7 @@ index ecda08f..ec630fe 100644
+ ++val; + ++val;
+ if (backw_stop != ~0ul) + if (backw_stop != ~0ul)
+ { + {
+ /* The is something pushed. */ + /* There is something pushed. */
+ if (backw == backw_stop) + if (backw == backw_stop)
+ { + {
+ /* The last pushed character was handled. Continue + /* The last pushed character was handled. Continue
@ -144,8 +144,8 @@ index ecda08f..ec630fe 100644
+ backw_stop = ~0ul; + backw_stop = ~0ul;
+ } + }
+ else + else
+ /* Nothing anymore. The backward sequence ended with + /* Nothing any more. The backward sequence ended with
+ the last sequence in the string. Note that seq2len + the last sequence in the string. Note that LEN
+ is still zero. */ + is still zero. */
+ break; + break;
+ } + }
@ -174,7 +174,7 @@ index ecda08f..ec630fe 100644
+ { + {
+ /* No sequence at all or just one. */ + /* No sequence at all or just one. */
+ if (idxcnt == idxmax || backw_stop > idxcnt) + if (idxcnt == idxmax || backw_stop > idxcnt)
+ /* Note that seq1len is still zero. */ + /* Note that LEN is still zero. */
+ break; + break;
+ +
+ backw_stop = ~0ul; + backw_stop = ~0ul;
@ -207,7 +207,7 @@ index ecda08f..ec630fe 100644
+ int pass) + int pass)
+{ +{
+#include WEIGHT_H +#include WEIGHT_H
+ int val = seq->val = 0; + size_t val = seq->val = 0;
+ int len = seq->len; + int len = seq->len;
+ size_t backw_stop = seq->backw_stop; + size_t backw_stop = seq->backw_stop;
+ size_t backw = seq->backw; + size_t backw = seq->backw;
@ -221,7 +221,7 @@ index ecda08f..ec630fe 100644
+ ++val; + ++val;
+ if (backw_stop != ~0ul) + if (backw_stop != ~0ul)
+ { + {
+ /* The is something pushed. */ + /* There is something pushed. */
+ if (backw == backw_stop) + if (backw == backw_stop)
+ { + {
+ /* The last pushed character was handled. Continue + /* The last pushed character was handled. Continue
@ -245,7 +245,7 @@ index ecda08f..ec630fe 100644
+ /* XXX Traverse BACKW sequences from the beginning of + /* XXX Traverse BACKW sequences from the beginning of
+ BACKW_STOP to get the next sequence. Is ther a quicker way + BACKW_STOP to get the next sequence. Is ther a quicker way
+ to do this? */ + to do this? */
+ int i = backw_stop; + size_t i = backw_stop;
+ us = seq->back_us; + us = seq->back_us;
+ while (i < backw) + while (i < backw)
+ { + {
@ -311,7 +311,7 @@ index ecda08f..ec630fe 100644
+ } + }
+ +
+ len = weights[idx++]; + len = weights[idx++];
+ /* Skip over indeces of previous levels. */ + /* Skip over indices of previous levels. */
+ for (int i = 0; i < pass; i++) + for (int i = 0; i < pass; i++)
+ { + {
+ idx += len; + idx += len;
@ -339,8 +339,8 @@ index ecda08f..ec630fe 100644
+{ +{
+ int seq1len = seq1->len; + int seq1len = seq1->len;
+ int seq2len = seq2->len; + int seq2len = seq2->len;
+ int val1 = seq1->val; + size_t val1 = seq1->val;
+ int val2 = seq2->val; + size_t val2 = seq2->val;
+ int idx1 = seq1->idx; + int idx1 = seq1->idx;
+ int idx2 = seq2->idx; + int idx2 = seq2->idx;
+ int result = 0; + int result = 0;
@ -348,7 +348,7 @@ index ecda08f..ec630fe 100644
+ /* Test for position if necessary. */ + /* Test for position if necessary. */
+ if (position && val1 != val2) + if (position && val1 != val2)
+ { + {
+ result = val1 - val2; + result = val1 > val2 ? 1 : -1;
+ goto out; + goto out;
+ } + }
+ +
@ -389,8 +389,8 @@ index ecda08f..ec630fe 100644
+{ +{
+ int seq1len = seq1->len; + int seq1len = seq1->len;
+ int seq2len = seq2->len; + int seq2len = seq2->len;
+ int val1 = seq1->val; + size_t val1 = seq1->val;
+ int val2 = seq2->val; + size_t val2 = seq2->val;
+ int32_t *idx1arr = seq1->idxarr; + int32_t *idx1arr = seq1->idxarr;
+ int32_t *idx2arr = seq2->idxarr; + int32_t *idx2arr = seq2->idxarr;
+ int idx1now = seq1->idxnow; + int idx1now = seq1->idxnow;
@ -400,7 +400,7 @@ index ecda08f..ec630fe 100644
+ /* Test for position if necessary. */ + /* Test for position if necessary. */
+ if (position && val1 != val2) + if (position && val1 != val2)
+ { + {
+ result = val1 - val2; + result = val1 > val2 ? 1 : -1;
+ goto out; + goto out;
+ } + }
+ +
@ -766,7 +766,7 @@ index ecda08f..ec630fe 100644
{ {
+ seq1.idxcnt = 0; + seq1.idxcnt = 0;
+ seq1.idx = 0; + seq1.idx = 0;
+ seq1.idx = 0; + seq2.idx = 0;
+ seq1.backw_stop = ~0ul; + seq1.backw_stop = ~0ul;
+ seq1.backw = ~0ul; + seq1.backw = ~0ul;
+ seq2.idxcnt = 0; + seq2.idxcnt = 0;
@ -774,7 +774,7 @@ index ecda08f..ec630fe 100644
+ seq2.backw = ~0ul; + seq2.backw = ~0ul;
+ +
+ /* We need the elements of the strings as unsigned values since they + /* We need the elements of the strings as unsigned values since they
+ are used as indeces. */ + are used as indices. */
+ seq1.us = (const USTRING_TYPE *) s1; + seq1.us = (const USTRING_TYPE *) s1;
+ seq2.us = (const USTRING_TYPE *) s2; + seq2.us = (const USTRING_TYPE *) s2;
+ +

View File

@ -1620,6 +1620,7 @@ rm -f *.filelist*
* Tue Aug 20 2013 Siddhesh Poyarekar <siddhesh@redhat.com> - 2.18-3 * Tue Aug 20 2013 Siddhesh Poyarekar <siddhesh@redhat.com> - 2.18-3
- Remove non-ELF support in rtkaio. - Remove non-ELF support in rtkaio.
- Avoid inlining of cleanup function for kaio_suspend. - Avoid inlining of cleanup function for kaio_suspend.
- Expand sizes of some types in strcoll (#855399, CVE-2012-4424).
* Mon Aug 19 2013 Siddhesh Poyarekar <siddhesh@redhat.com> - 2.18-2 * Mon Aug 19 2013 Siddhesh Poyarekar <siddhesh@redhat.com> - 2.18-2
- Fix buffer overflow in readdir_r (#995841, CVE-2013-4237). - Fix buffer overflow in readdir_r (#995841, CVE-2013-4237).