Stack address is returned from function translate_one

Bug #1661815 reported by shqking
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
QEMU
Fix Released
Undecided
Unassigned

Bug Description

The vulnerable version is qemu-2.8.0, and the vulnerable function is in "target-s390x/translate.c".

The code snippet is as following.

static ExitStatus translate_one(CPUS390XState *env, DisasContext *s)
{
    const DisasInsn *insn;
    ExitStatus ret = NO_EXIT;
    DisasFields f;
    ...
    s->fields = &f;
    ...
    s->pc = s->next_pc;
    return ret;
}

A stack address, i.e. the address of local variable "f" is returned from current function through the output parameter "s->fields" as a side effect.

This issue is one kind of undefined behaviors, according the C Standard, 6.2.4 [ISO/IEC 9899:2011] (https://www.securecoding.cert.org/confluence/display/c/DCL30-C.+Declare+objects+with+appropriate+storage+durations)

This dangerous defect may lead to an exploitable vulnerability.
We suggest sanitizing "s->fields" as null before return.

Note that this issue is reported by shqking and Zhenwei Zou together.

shqking (shqking)
tags: added: behavior undefined
tags: removed: behavior undefined
shqking (shqking)
Changed in qemu:
assignee: nobody → shqking (shqking)
assignee: shqking (shqking) → nobody
Revision history for this message
Thomas Huth (th-huth) wrote :

The calling function never uses "->fields", so I do not see a real vulnerability here, is there? Did you use a code analyser for this, or how did you come across this issue?

Revision history for this message
shqking (shqking) wrote :

Thanks for your reply.

Inspired by this issue in apache httpd (https://bz.apache.org/bugzilla/show_bug.cgi?id=59844#c0),
we customized a checker based on the Clang Static Analyzer to detect such undefined behavior.

Yes.
After examining the code carefully, we didn't find any place where the "->fields" is accessed, either. However, we think this kind of defect seems like a 'time bomb' and we'd better fix it just to be on the safe side.

Revision history for this message
Thomas Huth (th-huth) wrote :
Revision history for this message
Thomas Huth (th-huth) wrote :
Changed in qemu:
status: New → Fix Committed
Changed in qemu:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.