Project

General

Profile

Bug #1594

strxfrm may write past the specified buffer

Added by Rich Lowe almost 8 years ago. Updated over 7 years ago.

Status:
Resolved
Priority:
High
Category:
lib - userland libraries
Start date:
2011-10-04
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

Theo reports


#include <stdio.h>
#include <stdlib.h>
#include <strings.h>
#include <locale.h>

int main() {
  int size, nsize;
  char locale[128];
  unsigned char in[2] = { 0337, 0 };
  char *mout, *out, after, before;
  while(fgets(locale, sizeof(locale), stdin) != NULL) {
    locale[strlen(locale)-1] = '\0';
    printf("locale -> %s\n", locale);
    setlocale(LC_COLLATE, locale);
    size = strxfrm(NULL, (const char *)in, 0);

    printf("Wants %d\nLet's allocate only %d\n", size + 1, size);
    mout = malloc(size+2);
    out = mout + 1;
    /* This is an intentional illegal access */
    after = out[size];
    before = mout[0];
    nsize = strxfrm(out, (const char *)in, size);
    printf("srrxfrm -> %d\n", nsize);
    printf("clobbered the byte before? %d == %d?\n", before, mout[0]);
    printf("clobbered the byte after? %d == %d?\n", after, out[size]);
    if(before != mout[0] || after != out[size]) abort();
  }
}

aborts when the specified locale is el_GR.ISO8859-7

metropolis:~> echo el_GR.ISO8859-7 | ./foo
locale -> el_GR.ISO8859-7
Wants 2
Let's allocate only 1
srrxfrm -> 1
clobbered the byte before? -33 == -33?
clobbered the byte after? 0 == 46?
zsh: done                           echo el_GR.ISO8859-7 | 
zsh: IOT instruction (core dumped)  ./foo

He proposes

diff -r da58f9d4ef03 usr/src/lib/libc/port/locale/collate.c
--- a/usr/src/lib/libc/port/locale/collate.c    Mon Aug 22 11:37:10 2011 -0400
+++ b/usr/src/lib/libc/port/locale/collate.c    Tue Oct 04 07:38:45 2011 -0700
@@ -588,9 +588,10 @@
                                if (room) {
                                        while (b) {
                                                b--;
-                                               if (room)
+                                               if (room) {
                                                        *xf++ = buf[b];
-                                               room--;
+                                                       room--;
+                                               }
                                        }
                                }
                                need = want;
@@ -613,9 +614,10 @@

                                        while (b) {
                                                b--;
-                                               if (room)
+                                               if (room) {
                                                        *xf++ = buf[b];
-                                               room--;
+                                                       room--;
+                                               }
                                        }
                                }
                                need = want;

as a fix.

I (separately) am also suspicious by the length returned. On the mac, we think this will require 13 bytes, rather than 2.

Also, and perhaps relatedly in strxfrm slen is in units of sizeof(wchar_t), surely the return should multiply so it's in bytes as returned? But maybe not, I'm tired.

History

#1

Updated by Garrett D'Amore over 7 years ago

  • Assignee set to Garrett D'Amore

Since this is my buggy code, I'm going to take it.

#2

Updated by Garrett D'Amore over 7 years ago

So Theo's fix is good.

But your other concerns are wrong. My enhancements compress the strxfrm'd string quite nicely, so 2 bytes to represent the sort order is probably quite enough. In fact for a simple 8 bit character set without any unusual sorting rules, it is guaranteed to be enough.

The slen is a count in characters of the source string in strxfrm, and is only used in the event that we can't complete a transformation. (strxfrm has no way to return an error, and conventionally we just return the original string.) Anyway, this should not occur for proper inputs.

#3

Updated by Garrett D'Amore over 7 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100
  • Tags deleted (needs-triage)

Resolved in:

changeset: 13588:4df486ad9a31
tag: tip
user: Theo Schlossnagle <>
date: Wed Feb 01 19:59:23 2012 -0800
description:
1594 strxfrm may write past the specified buffer
Reviewed by: Garrett D'Amore <>
Reviewed by: Richard Lowe <>
Reviewed by: Adam Leventhal <>
Reviewed by: Dan McDonald <>
Approved by: Garrett D'Amore <>

Also available in: Atom PDF