Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Null pointer dereference – new security bug for OpenSSL

ftp.openbsd.org

Post reply on HN
Print view View original

31–38 of 38 posts

Re: Null pointer dereference – new security bug for OpenSSL

#31

Post by ams6110 »

That patch is set up for a later bug to be introduced: no brackets on the if statements.

Instead of:

  +		if (wb->buf == NULL)
  +			if (!ssl3_setup_write_buffer(s))
  +				return -1;
Why not:

  +		if (wb->buf == NULL) {
  +			if (!ssl3_setup_write_buffer(s)) {
  +				return -1;
  +			}
  +		}

Re: Null pointer dereference – new security bug for OpenSSL

#32

Post by codezero »

ams6110 wrote: post #31

That patch is set up for a later bug to be introduced: no brackets on the if statements. Instead of: + if (wb->buf == NULL) + if (!ssl3_setup_write_buffer(s)) + return -1; Why not: + if (wb->buf == NULL) { + if (!ssl3_setup_write_buffer(s)) { + return -1; + } + }

or

  if ((wb->buf == NULL) && !ssl3_setup_write_buffer(s)) {
     return -1;
  }

Re: Null pointer dereference – new security bug for OpenSSL

#33

Post by insaneirish »

ams6110 wrote: post #31

That patch is set up for a later bug to be introduced: no brackets on the if statements. Instead of: + if (wb->buf == NULL) + if (!ssl3_setup_write_buffer(s)) + return -1; Why not: + if (wb->buf == NULL) { + if (!ssl3_setup_write_buffer(s)) { + return -1; + } + }

This is OpenBSD Kernel Normal Form (http://www.openbsd.org/cgi-bin/man.cgi?query=style&sektion=9).

Disagree you may, but it's consistent with their self-imposed guidelines.

Re: Null pointer dereference – new security bug for OpenSSL

#34

Post by ibotty »

ams6110 wrote: post #31

That patch is set up for a later bug to be introduced: no brackets on the if statements. Instead of: + if (wb->buf == NULL) + if (!ssl3_setup_write_buffer(s)) + return -1; Why not: + if (wb->buf == NULL) { + if (!ssl3_setup_write_buffer(s)) { + return -1; + } + }

this is getting boring. ever time a patch using no parentheses shows up there is that comment. if all your code uses that style you get used to it. feel free to show evidence of openbsd making that mistake once! (there might have been those, but i am sure it's going to be very hard to find).

Re: Null pointer dereference – new security bug for OpenSSL

#35

Post by Qantourisc »

Qantourisc wrote: post #15

Is it me or should code that has to be secure be written in more manage languages to prevent these mistake ? (But managed languages probably have other security issues I don't know about ?)

For all clarity I meant like C++, ObjC, or others; any language that allows you to create things that enforce consistency during compile or runtime. (Like shared pointer and array containers.) Probably used the wrong terminology here.

Re: Null pointer dereference – new security bug for OpenSSL

#36

Post by nextw33k »

ibotty wrote: post #34
ams6110 wrote: post #31

That patch is set up for a later bug to be introduced: no brackets on the if statements. Instead of: + if (wb->buf == NULL) + if (!ssl3_setup_write_buffer(s)) + return -1; Why not: + if (wb->buf == NULL) { + if (!ssl3_setup_write_buffer(s)) { + return -1; + } + }

this is getting boring. ever time a patch using no parentheses shows up there is that comment. if all your code uses that style you get used to it. feel free to show evidence of openbsd making that mistake once! (there might have been those, but i am sure it's going to be very hard to find).

Perhaps you need to consider the bigger picture. The biggest part of being a programmer is not in how many characters we write but in how we are able to know and understand abstract concepts.

If writing defensively by always putting braces around if statements means we no longer have to consider the possibility of a fall through error, then that saves mental processing for you and more importantly for lesser programmers.

Re: Null pointer dereference – new security bug for OpenSSL

#38

Post by codezero »

insaneirish wrote: post #33
ams6110 wrote: post #31

That patch is set up for a later bug to be introduced: no brackets on the if statements. Instead of: + if (wb->buf == NULL) + if (!ssl3_setup_write_buffer(s)) + return -1; Why not: + if (wb->buf == NULL) { + if (!ssl3_setup_write_buffer(s)) { + return -1; + } + }

This is OpenBSD Kernel Normal Form ( http://www.openbsd.org/cgi-bin/man.cgi?query=style&sektion=9 ). Disagree you may, but it's consistent with their self-imposed guidelines.

Sort of. They say only if it's a single line that it's not allowed. If there are multiple lines it's "permitted."

So technically, this is still OK, but they are allowed to use braces when there are multiple lines. (the if and the return) They have an example in that link that covers this as a permitted case, but as you say, it's OK and consistent, but that doesn't make it good :)

Post reply on HN