New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

undefined behaviour in archive_read_support_format_mtree.c #539

Closed
hannob opened this Issue May 12, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@hannob
Contributor

hannob commented May 12, 2015

When compiling libarchive with the compile flag -fsanitize=undefined (enabling undefined behaviour sanitizer) it'll throw a warning when trying to open any mtree file:
libarchive/archive_read_support_format_mtree.c:146:10: runtime error: signed integer overflow: 9223372036854775807 * 2 cannot be represented in type 'long int'
libarchive/archive_read_support_format_mtree.c:168:6: runtime error: signed integer overflow: -9223372036854775808 * 2 cannot be represented in type 'long int'

This is the code that's causing this:
static int64_t
get_time_t_max(void)
{

if defined(TIME_T_MAX)

return TIME_T_MAX;

else

static time_t t;
time_t a;
if (t == 0) {
    a = 1;
    while (a > t) {
        t = a;
        a = a * 2 + 1;
    }
}
return t;

endif

}

static int64_t
get_time_t_min(void)
{

if defined(TIME_T_MIN)

return TIME_T_MIN;

else

/* 't' will hold the minimum value, which will be zero (if
 * time_t is unsigned) or -2^n (if time_t is signed). */
static int computed;
static time_t t;
time_t a;
if (computed == 0) {
    a = (time_t)-1;
    while (a < t) {
        t = a;
        a = a * 2;
    }           
    computed = 1;
}
return t;

endif

}

What libarchive is trying to do here is calculating the value of TIME_T_MIN/MAX by triggering an overflow.

However overflows in signed values are undefined in C. This code is therefore strictly speaking invalid, the compiler may do whatever it likes in such situations, without any defined outcome.

I haven't come up with an elegant other way to do this yet. Probably the best would be to convince the glibc devs to define TIME_T_MIN/MAX in their headers.

@kientzle

This comment has been minimized.

Show comment
Hide comment
@kientzle

kientzle May 16, 2015

Contributor

We run on a lot of platforms that don't use glibc, so we need something more portable than that.

Commit b31744d reworks this code to use a different approach. I believe this avoids undefined behaviors.

Contributor

kientzle commented May 16, 2015

We run on a lot of platforms that don't use glibc, so we need something more portable than that.

Commit b31744d reworks this code to use a different approach. I believe this avoids undefined behaviors.

@kientzle kientzle closed this May 16, 2015

@petterreinholdtsen

This comment has been minimized.

Show comment
Hide comment
@petterreinholdtsen

petterreinholdtsen Jun 29, 2016

According to <URL: https://security-tracker.debian.org/tracker/CVE-2015-8931 > this is a security issue with ID CVE-2015-8931 .

petterreinholdtsen commented Jun 29, 2016

According to <URL: https://security-tracker.debian.org/tracker/CVE-2015-8931 > this is a security issue with ID CVE-2015-8931 .

@petterreinholdtsen

This comment has been minimized.

Show comment
Hide comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment