Lighttpd

stbuehler wrote:

Hi!

zhenbxoxu wrote:

Hi,
I'm a developer of a static analysis tool canalyze.
Recently I applied it to lighttpd-1.4.32.

Thanks :)

It seems some reports are real after by manually checing:
1. Use Undefined Value
file: mod_redirect.c
function: SETDEFAULTS_FUNC
At line 86: data_array *da = (data_array *)du;
du is undefined value.

Yes... otoh, the assigned undefined value is never actually used.

2. Use Undefined Value
file: http_auth.c
function: base64_decode
At line 95: buffer_prepare_copy(out, in_len);
out->ptr may be a newly allocated memory object.

At line 109: if (ch < 0) continue;
ch < 0 is satifiable at the first iteration, i.e. i 0

And at the second iteration, i 1
At line 116: result[j++] |= ch >> 4;
result[j] is a undefined value.

Oh this looks ugly...
As it skips invalid characters (I think we could just fail on invalid characters) it should keep the "state % 4" in a separate var.
Also the padding seems to insert a 0 character as data (it counts to out->used).

I cannot see any insertion in the for loop. Could you explain it in more detail?

3. Memory leak
file: mod_simple_vhost.c
function: build_doc_root
At line 166: Do we need to free the buffer in out?

I don't see a buffer allocation, why would we free one? The buffer gets reused, no need to empty it every time.

4. Memory leak
file: proc_open.c
function: proc_open_buffer
At line 312: buffer *tmp = buffer_init();
Create a new buffer here.

At line 320: Do we need to free the buffer when error writing pipe.

We could. Otoh, the error in this place will make lighttpd exit anyway (only used for config include_shell).

5. Memory leak
file: fdevent.c
function: fdevent_init
At line 68: if (0 != fdevent_libev_init(ev))
"ev" is not released when event-handler libev failed.

Same as 4 - without an event-handler lighttpd won't start.

6. Memory leak
file: lemon.c
function: tplt_open
At line 2867: tpltname = pathsearch(lemp->argv0,lemp->tmplname,0);
Memory is allocatd here but not released when return.

Just a parser generator, only used for building. There are probably newer versions of lemon.

7. Use After free
file: mod_fastcgi.c
function: fastcgi_get_packet
At line 2426: buffer_free(packet->b);
Buffer is freed and is used
At line 2429: log_error_write(srv, FILE, LINE, "sdsds", "FastCGI: header too small:", packet->b->used, "bytes <", sizeof(FCGI_Header), "bytes, waiting for more data");

Right. Only debug option and FastCGI backends have other ways to cause DoS (OOM trigger by sending large responses to slow clients) => not a security issue.

8. Memory leak
file: network_linux_sendfile.c
function: network_write_chunkqueue_linuxsendfile
At line 180: if (HANDLER_ERROR == stat_cache_get_entry(srv, con, c->file.name, &sce)) {
Memory is allocated to sce,

Should we release "sce" at line 189 and 193?

No. stat cache entries are kept in a tree for later reuse. The mainloop triggers cleaning each second.

9. Memory leak
file: lemon.c
function: FindStates
At line 744: (void)getstate(lemp);
would return a heap object

See 6.

And it also reports hundreds of null pointer dereferences caused by unchecked malloc failure. For example:
file: array.c
function: array_init_array
At line 32: a->data = malloc(...)
At line 34: if (src->data[i]) a->data[i] = src->data[i]->copy(src->data[i]);
use a->data without failure checking.

We could insert more assertions of course, but the result will be the same - lighttpd will die on OOM.

OOM? Not Segment Fault?

Hope for your replies! Thank you!

Regards,
Zhenbo Xu

Thank you very much for your input!

2. Use Undefined Value
file: http_auth.c
function: base64_decode
At line 95: buffer_prepare_copy(out, in_len);
out->ptr may be a newly allocated memory object.

At line 109: if (ch < 0) continue;
ch < 0 is satifiable at the first iteration, i.e. i 0

And at the second iteration, i 1
At line 116: result[j++] |= ch >> 4;
result[j] is a undefined value.

Oh this looks ugly...
As it skips invalid characters (I think we could just fail on invalid characters) it should keep the "state % 4" in a separate var.
Also the padding seems to insert a 0 character as data (it counts to out->used).

I cannot see any insertion in the for loop. Could you explain it in more detail?

The padding insert is at the end, not in the loop. Just comments to myself what i should fix :)

And it also reports hundreds of null pointer dereferences caused by unchecked malloc failure. For example:
file: array.c
function: array_init_array
At line 32: a->data = malloc(...)
At line 34: if (src->data[i]) a->data[i] = src->data[i]->copy(src->data[i]);
use a->data without failure checking.

We could insert more assertions of course, but the result will be the same - lighttpd will die on OOM.

OOM? Not Segment Fault?

OOM == Out of Memory. And either it dies with an abort() from the assert() or with a segfault - not a huge difference (as long as it actually segfaults; NULL from alloc could also lead to memory corruption if you only use memory at high offsets from the base pointer).