#1114 get_uid_from_pid() perfoms an improper read
Closed: Fixed None Opened 8 years ago by sgallagh.

    char buf[BUFSIZE];
    while ((ret = read(fd, buf, BUFSIZE)) != 0) {
        if (ret == -1) {
            error = errno;
            if (error == EINTR || error == EAGAIN) {
            DEBUG(1, ("read failed [%d][%s].\n", error, strerror(error)));
            goto fail_fd;

It's theoretically possible for {{{read()}}} to overrun BUFSIZE here (though it's incredibly unlikely, since we're reading from /proc, so the only way to exploit this file would be a kernel exploit)

This should be rewritten to shrink the request BUFSIZE on subsequent passes to read(). We also need to add an explicit NULL-terminator.

Also, note that buf isn't being advanced after a partial read, so it could be overwriting buf.

We should create a common sss_read() function somewhere that handles reading in a safe way, always.

Fields changed

blockedby: =>
blocking: =>

"Nice to have" for 1.9.

milestone: SSSD 1.9.0 NEEDS_TRIAGE => SSSD 1.9.0

Fields changed

rhbz: => 0

Fields changed

feature_milestone: =>
keywords: Coverity => Coverity easyfix

Fixed by 9d7d445

milestone: SSSD 1.9.0 => SSSD 1.9.0 beta 1
resolution: => fixed
status: new => closed

Metadata Update from @sgallagh:
- Issue assigned to jhrozek
- Issue set to the milestone: SSSD 1.9.0 beta 1

2 years ago

Login to comment on this ticket.