207926 2008-01-28 16:51 0000 sys-devel/gcc-config has buffer overflows 2008-03-16 01:20:25 0000 1 1 1 Unclassified Gentoo Linux Core system unspecified All Linux RESOLVED FIXED P2 minor --- 1 eteran@alum.rit.edu toolchain@gentoo.org tcunha@gentoo.org eteran@alum.rit.edu 2008-01-28 16:51:33 0000 I was looking into why gentoo uses gcc-config instead of an "eselect-gcc" which would be more consistant with the rest of gentoo. So I figured that I would look at the gcc-config sources, after all, it essentially does similar work to eselect-gcc, perhaps I could help :) Anyway, I discovered misuse of several functions causing buffer overflows. My examples are from wrapper-1.5.0.c: line 328: "strcpy(data.name, basename(argv[0]));" more hypothetical than practical, but since you don't know the length of argv[0], it should be a strncpy. lines 218-221: "strncpy(data->bin, str, sizeof(data->bin) - 1); data->bin[strlen(data->bin) - 1] = '/'; strncat(data->bin, data->name, sizeof(data->bin) - 1); data->bin[MAXPATHLEN] = 0;" here, the problem is misuse of the strncat function. The size parameter is supposed to represent how much is LEFT in the buffer (total size - used part). Not the total size. I will attach a program which demonstrates that it does in fact cause an overflow. Fortunately, the only damage that is done is the data->tmp variable gets trashed. However, this whole thing could be safely replaced with something like this: snprintf(data->bin, sizeof(data->bin), "%s/%s", str, data->name); line 48: strncpy does not gaurantee null termination for long strings line 71-73: "size_t len = strlen(path) + strlen(data->name) + 2; snprintf(str, len, "%s/%s", path, data->name);" The size param of snprintf should be the size of the buffer, not the size of the source data! This code basically does no bounds checking at all, because you set the len equal to the size of the source data. These should be easy to clean up, but of course I am still interested in my end gaol of making gcc-config more consistent with the eselect system. But that is for another day :) Thanks, Evan Teran Reproducible: Always eteran@alum.rit.edu 2008-01-28 16:52:21 0000 Created an attachment (id=142028) example of overflow vapier@gentoo.org 2008-03-16 01:20:25 0000 eselect-gcc is a duplicate of gcc-config at the moment, not the other way around string handling has been cleaned up in gcc-config-1.4.1: http://sources.gentoo.org/sys-devel/gcc-config/files/wrapper-1.5.1.c?rev=1.1 142028 2008-01-28 16:52 0000 example of overflow test.c text/plain CiNkZWZpbmUgX1JFRU5UUkFOVAojZGVmaW5lIF9HTlVfU09VUkNFCgojZGVmaW5lIE1BWFBBVEhM RU4gNDA5NgoKI2luY2x1ZGUgPHN0ZGlvLmg+CiNpbmNsdWRlIDxzdGRsaWIuaD4KI2luY2x1ZGUg PHN5cy90eXBlcy5oPgojaW5jbHVkZSA8c3lzL3N0YXQuaD4KI2luY2x1ZGUgPHN5cy9wYXJhbS5o PgojaW5jbHVkZSA8dW5pc3RkLmg+CiNpbmNsdWRlIDxzeXMvd2FpdC5oPgojaW5jbHVkZSA8bGli Z2VuLmg+CiNpbmNsdWRlIDxzdHJpbmcuaD4KI2luY2x1ZGUgPHN0ZGFyZy5oPgojaW5jbHVkZSA8 ZXJybm8uaD4KCi8qIGN1dCBmcm9tIHdyYXBwZXItMS41LjAuYyAqLwpzdHJ1Y3Qgd3JhcHBlcl9k YXRhIHsKCWNoYXIgbmFtZVtNQVhQQVRITEVOICsgMV07CgljaGFyIGZ1bGxuYW1lW01BWFBBVEhM RU4gKyAxXTsKCWNoYXIgYmluW01BWFBBVEhMRU4gKyAxXTsKCWNoYXIgdG1wW01BWFBBVEhMRU4g KyAxXTsKCWNoYXIgKnBhdGg7Cn07CgppbnQgbWFpbigpIHsKCgkvKiBzZXR1cCB0ZXN0ICovCglz dHJ1Y3Qgd3JhcHBlcl9kYXRhIGQ7CglzdHJ1Y3Qgd3JhcHBlcl9kYXRhICpkYXRhID0gJmQ7CgkK CW1lbXNldChkYXRhLCAwLCBzaXplb2YoZGF0YSkpOwoJCgljaGFyIHN0cltNQVhQQVRITEVOXTsK CW1lbXNldChzdHIsICdBJywgc2l6ZW9mKHN0cikpOwoJc3RyW01BWFBBVEhMRU5dID0gJ1wwJzsK CW1lbXNldChkYXRhLT5uYW1lLCAnQicsIHNpemVvZihkYXRhLT5uYW1lKSk7CglkYXRhLT5uYW1l W01BWFBBVEhMRU5dID0gJ1wwJzsKCQoJc3RyY3B5KGRhdGEtPnRtcCwgImhlbGxvIHdvcmxkIik7 CgoKCgojZGVmaW5lIFRFU1QKI2lmZGVmIFRFU1QKCS8qIGRvIHRlc3QsIHRoaXMgY29kZSB3YXMg Y29waWVkIGRpcmVjdGx5IGZyb20gdGhlIHdyYXBwZXItMS41LjAuYyAKCSAqIGxpbmVzIDIxOCAt IDIyMQoJICovCglzdHJuY3B5KGRhdGEtPmJpbiwgc3RyLCBzaXplb2YoZGF0YS0+YmluKSAtIDEp OwoJZGF0YS0+YmluW3N0cmxlbihkYXRhLT5iaW4pIC0gMV0gPSAnLyc7CglzdHJuY2F0KGRhdGEt PmJpbiwgZGF0YS0+bmFtZSwgc2l6ZW9mKGRhdGEtPmJpbikgLSAxKTsKCWRhdGEtPmJpbltNQVhQ QVRITEVOXSA9IDA7CiNlbmRpZgoKCS8qIHJlc3VsdHMgKi8KCXByaW50Zigic3RybGVuKGRhdGEt Pm5hbWUpID0gJWQgOiBzaG91bGQgYmUgJWRcbiIsIHN0cmxlbihkYXRhLT5uYW1lKSwgNDA5Nik7 CglwcmludGYoInN0cmxlbihkYXRhLT5mdWxsbmFtZSkgPSAlZCA6IHNob3VsZCBiZSAlZFxuIiwg c3RybGVuKGRhdGEtPmZ1bGxuYW1lKSwgMCk7CglwcmludGYoInN0cmxlbihkYXRhLT5iaW4pID0g JWQgOiBzaG91bGQgYmUgJWRcbiIsIHN0cmxlbihkYXRhLT5iaW4pLCA0MDk2KTsKCXByaW50Zigi c3RybGVuKGRhdGEtPnRtcCkgPSAlZCA6IHNob3VsZCBiZSAlZFxuIiwgc3RybGVuKGRhdGEtPnRt cCksIDExKTsKCXByaW50ZigiZGF0YS0+cGF0aCA9ICVwIDogc2hvdWxkIGJlIE5VTExcbiIsIGRh dGEtPnBhdGgpOwoKfQo=