Use after free in SSL_free and SSL_set_bio about openbsd HOT 12 CLOSED

Checking the freed pointer value is not a use-after-free. The pointer would have to be dereferenced first, and it is not here.

from openbsd.

By this logic, What would you say is the set of possible outputs of the following program?

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

int main(void) {
  int *p = (int*)malloc(sizeof(int));
  int *q = (int*)realloc(p, sizeof(int));
  *p = 1;
  *q = 2;
  if (p == q) printf("%d %d\n", *p, *q);
}

It seems to me that if “checking [a] freed pointer value is not a use-after-free”, then a program that contains the line if (p == q) printf("%d %d\n", *p, *q); should only every print two identical values.

The above program, when compiled with Clang, prints “1 2”.

A dangling pointer is indeterminate. Using indeterminate memory for anything is undefined behavior. The above program invokes undefined behavior. It can do anything.

SSL_free and SSL_set_bio invoke undefined behavior for exactly the same reason as the above example. They can do anything for exactly the same reason.

See also: A dangling pointer is indeterminate. I co-wrote that post, I am not claiming that it is a second opinion, I am claiming that it contains examples that you can reproduce with your own version of GCC and/or Clang, the version that thousands of people will use to compile LibreSSL, and if the code compares dangling pointers, these compilers do not have enough respect for it to generate a binary that follows the rules of logic.

from openbsd.

man 3 realloc please...

from openbsd.

It has been pointed out to me that the following modified example makes a stronger point (modification by Jesse Ruderman).

int main() {
int *p = (int*)malloc(sizeof(int));
if (p == NULL) { return 3; }
int *q = (int*)realloc(p, sizeof(int));
if (q == NULL) { return 3; }
*q = 2;
if (p == q) {
*p = 1;
printf(“%d %d\n”, *p, *q);
}
}

clang -Weverything -O r.c && ./a.out
1 2

from openbsd.

@bob-beck I don't know which man page you are referring to, but since LibreSSL aims to sane POSIX platforms, perhaps we can use the C99 standard instead?

J.2 Undefined behavior

…
— The value of a pointer that refers to space deallocated by a call to the free or realloc function is used (7.20.3).

from openbsd.

"is used to access an object" is the key bit of that phrase. That isn't what's going on in your original code example.

from openbsd.

I temporarily copy-pasted the wrong line, sorry about that (too many references to realloc in J.2). “is used” is the key bit of the updated quote.

from openbsd.

brent I'm not convinced this isn't an issue.. I'm re-opening until we decide for certain

from openbsd.

At a minimum it's ugly as heck and should be redone more sanely. but this is an area to tread cautiously in.

from openbsd.

OK, now that you actually explained the issue, it makes more sense. Agreed it's certainly ugly regardless of whether it's correct.

from openbsd.

On Thu, Oct 15, 2015 at 04:26:37PM -0700, Brent Cook wrote:

OK, now that you actually explained the issue, it makes more sense. Agreed it's certainly ugly regardless of whether it's correct.

I have diffs mailed for it.

---

Reply to this email directly or view it on GitHub:
#54 (comment)

from openbsd.

Fix commited - thanks

from openbsd.