[#855399] Fix to strcoll CVE patch
by Siddhesh Poyarekar
Hi,
I'd like to add this fix to the strcoll CVE patch to resolve a crash
that may occur in strcoll in the non-cached version when the length
overflows int32_t. All it does is expand sizes for a couple of
variables so that they don't overflow. OK to commit?
Siddhesh
commit 735547c10eb2f3981b702fbb84d36934630dc185
Author: Siddhesh Poyarekar <siddhesh(a)redhat.com>
Date: Thu Aug 22 11:10:25 2013 +0530
Fix an issue with the strcoll CVE patch
Expanded types for some variables to prevent overflow.
diff --git a/glibc-strcoll-cve.patch b/glibc-strcoll-cve.patch
index 98ac2e6..5558c1f 100644
--- a/glibc-strcoll-cve.patch
+++ b/glibc-strcoll-cve.patch
@@ -1,5 +1,5 @@
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
+++ b/string/strcoll_l.c
@@ -41,11 +41,434 @@
@@ -10,15 +10,15 @@ index ecda08f..ec630fe 100644
+typedef struct
+{
+ 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. */
+ size_t idxnow; /* Current 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_stop; /* Index where the backward sequences stop. */
+ 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 rule; /* Saved rule for the first 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. */
+} 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. */
+static void
+get_next_seq_cached (coll_seq *seq, int nrules, int pass,
+ const unsigned char *rulesets,
+ const USTRING_TYPE *weights)
+{
-+ int val = seq->val = 0;
++ size_t val = seq->val = 0;
+ int len = seq->len;
+ size_t backw_stop = seq->backw_stop;
+ size_t backw = seq->backw;
@@ -50,7 +50,7 @@ index ecda08f..ec630fe 100644
+ ++val;
+ if (backw_stop != ~0ul)
+ {
-+ /* The is something pushed. */
++ /* There is something pushed. */
+ if (backw == backw_stop)
+ {
+ /* The last pushed character was handled. Continue
@@ -62,7 +62,7 @@ index ecda08f..ec630fe 100644
+ }
+ else
+ {
-+ /* Nothing anymore. The backward sequence
++ /* Nothing any more. The backward sequence
+ ended with the last sequence in the string. */
+ idxnow = ~0ul;
+ break;
@@ -88,7 +88,7 @@ index ecda08f..ec630fe 100644
+ {
+ /* No sequence at all or just one. */
+ if (idxcnt == idxmax)
-+ /* Note that seq1len is still zero. */
++ /* Note that LEN is still zero. */
+ break;
+
+ backw_stop = ~0ul;
@@ -117,7 +117,7 @@ index ecda08f..ec630fe 100644
+ const USTRING_TYPE *extra, const int32_t *indirect)
+{
+#include WEIGHT_H
-+ int val = seq->val = 0;
++ size_t val = seq->val = 0;
+ int len = seq->len;
+ size_t backw_stop = seq->backw_stop;
+ size_t backw = seq->backw;
@@ -133,7 +133,7 @@ index ecda08f..ec630fe 100644
+ ++val;
+ if (backw_stop != ~0ul)
+ {
-+ /* The is something pushed. */
++ /* There is something pushed. */
+ if (backw == backw_stop)
+ {
+ /* The last pushed character was handled. Continue
@@ -144,8 +144,8 @@ index ecda08f..ec630fe 100644
+ backw_stop = ~0ul;
+ }
+ else
-+ /* Nothing anymore. The backward sequence ended with
-+ the last sequence in the string. Note that seq2len
++ /* Nothing any more. The backward sequence ended with
++ the last sequence in the string. Note that LEN
+ is still zero. */
+ break;
+ }
@@ -174,7 +174,7 @@ index ecda08f..ec630fe 100644
+ {
+ /* No sequence at all or just one. */
+ if (idxcnt == idxmax || backw_stop > idxcnt)
-+ /* Note that seq1len is still zero. */
++ /* Note that LEN is still zero. */
+ break;
+
+ backw_stop = ~0ul;
@@ -207,7 +207,7 @@ index ecda08f..ec630fe 100644
+ int pass)
+{
+#include WEIGHT_H
-+ int val = seq->val = 0;
++ size_t val = seq->val = 0;
+ int len = seq->len;
+ size_t backw_stop = seq->backw_stop;
+ size_t backw = seq->backw;
@@ -221,7 +221,7 @@ index ecda08f..ec630fe 100644
+ ++val;
+ if (backw_stop != ~0ul)
+ {
-+ /* The is something pushed. */
++ /* There is something pushed. */
+ if (backw == backw_stop)
+ {
+ /* The last pushed character was handled. Continue
@@ -245,7 +245,7 @@ index ecda08f..ec630fe 100644
+ /* XXX Traverse BACKW sequences from the beginning of
+ BACKW_STOP to get the next sequence. Is ther a quicker way
+ to do this? */
-+ int i = backw_stop;
++ size_t i = backw_stop;
+ us = seq->back_us;
+ while (i < backw)
+ {
@@ -311,7 +311,7 @@ index ecda08f..ec630fe 100644
+ }
+
+ len = weights[idx++];
-+ /* Skip over indeces of previous levels. */
++ /* Skip over indices of previous levels. */
+ for (int i = 0; i < pass; i++)
+ {
+ idx += len;
@@ -339,8 +339,8 @@ index ecda08f..ec630fe 100644
+{
+ int seq1len = seq1->len;
+ int seq2len = seq2->len;
-+ int val1 = seq1->val;
-+ int val2 = seq2->val;
++ size_t val1 = seq1->val;
++ size_t val2 = seq2->val;
+ int idx1 = seq1->idx;
+ int idx2 = seq2->idx;
+ int result = 0;
@@ -348,7 +348,7 @@ index ecda08f..ec630fe 100644
+ /* Test for position if necessary. */
+ if (position && val1 != val2)
+ {
-+ result = val1 - val2;
++ result = val1 > val2 ? 1 : -1;
+ goto out;
+ }
+
@@ -389,8 +389,8 @@ index ecda08f..ec630fe 100644
+{
+ int seq1len = seq1->len;
+ int seq2len = seq2->len;
-+ int val1 = seq1->val;
-+ int val2 = seq2->val;
++ size_t val1 = seq1->val;
++ size_t val2 = seq2->val;
+ int32_t *idx1arr = seq1->idxarr;
+ int32_t *idx2arr = seq2->idxarr;
+ int idx1now = seq1->idxnow;
@@ -400,7 +400,7 @@ index ecda08f..ec630fe 100644
+ /* Test for position if necessary. */
+ if (position && val1 != val2)
+ {
-+ result = val1 - val2;
++ result = val1 > val2 ? 1 : -1;
+ goto out;
+ }
+
@@ -766,7 +766,7 @@ index ecda08f..ec630fe 100644
{
+ seq1.idxcnt = 0;
+ seq1.idx = 0;
-+ seq1.idx = 0;
++ seq2.idx = 0;
+ seq1.backw_stop = ~0ul;
+ seq1.backw = ~0ul;
+ seq2.idxcnt = 0;
@@ -774,7 +774,7 @@ index ecda08f..ec630fe 100644
+ seq2.backw = ~0ul;
+
+ /* 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;
+ seq2.us = (const USTRING_TYPE *) s2;
+
diff --git a/glibc.spec b/glibc.spec
index 8615ffa..d5df9dd 100644
--- a/glibc.spec
+++ b/glibc.spec
@@ -1620,6 +1620,7 @@ rm -f *.filelist*
* Tue Aug 20 2013 Siddhesh Poyarekar <siddhesh(a)redhat.com> - 2.18-3
- Remove non-ELF support in rtkaio.
- 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(a)redhat.com> - 2.18-2
- Fix buffer overflow in readdir_r (#995841, CVE-2013-4237).
10 years, 8 months
[PATCH] Complete `Move to /usr' for glibc.
by Carlos O'Donell
The GNU C Library package never completed the `Move to /usr' transition.
This failure to complete the transition means that the package still
reports that it owns files in /lib64 instead of /usr/lib64 and that
can cause problems for the tools that consume and use the rpm db.
It is about time that the glibc.spec file had a serious fixup and the
failure to complete the `Move to /usr' transition was the impetus that
led me to do the cleanup.
The cleanup leading to the `Move to /usr' transition was broken out
into a set of ~15 patches that are already checked in.
The following patch completes the `Move to /usr' transition for glibc.
It also ensures that `/usr' is always written as %{_prefix} for glibc
files, though references to already installed tools are not changed
to use %{_prefix} (since that would lead to a bootstrap problem when
changing _prefix).
Committed (with typo in commit message s/not/now/g :/)
commit 62c3082486aa6e0341a978c15ca3903802c58a81
Author: Carlos O'Donell <carlos(a)redhat.com>
Date: Tue Aug 13 13:18:15 2013 -0400
Complete `Move to /usr' transition for package.
All relevant files are not installed to /usr, with the expectation
that the distribution will provide compatibility links from the
old paths to the new paths. All uses of a bare `/usr' have been
replaced with uses of `%{_prefix}' for files installed by the
package.
Cheers,
Carlos.
10 years, 8 months