1252949 - graphite2: UBSan load of value which is not valid for type 'const graphite2::vm::opcode' [@graphite2::vm::Machine::Code::decoder::fetch_opcode]

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.ttf

This was found while fuzzing graphite2 latest revision (bc5409c573aa9ecccacd18cf713021272998cd35)

This issue was uncovered using Undefined Behavior Sanitizer (UBSan). More information can be found here: http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html.

This is likely not a sec issue however I am hiding this bug because of the large number of bugs that have been found and I would like to avoid any unwanted attention until things calm down.

To reproduce:
Build with UBSan enabled.
run: ./gr2fonttest test_case.ttf -auto

/home/user/code/graphite/src/Code.cpp:277:26: runtime error: load of value 153, which is not a valid value for type 'const graphite2::vm::opcode'
    #0 0x7f9856ab4657 in graphite2::vm::Machine::Code::decoder::fetch_opcode(unsigned char const*) /home/user/code/graphite/src/Code.cpp:277:26
    #1 0x7f9856ab1cb3 in graphite2::vm::Machine::Code::decoder::load(unsigned char const*, unsigned char const*) /home/user/code/graphite/src/Code.cpp:256:28
    #2 0x7f9856aae608 in graphite2::vm::Machine::Code::Code(bool, unsigned char const*, unsigned char const*, unsigned char, unsigned short, graphite2::Silf const&, graphite2::Face const&, graphite2::passtype, unsigned char**) /home/user/code/graphite/src/Code.cpp:196:9
    #3 0x7f9856be1e3c in graphite2::Pass::readRules(unsigned char const*, unsigned long, unsigned char const*, unsigned short const*, unsigned short const*, unsigned char const*, unsigned short const*, unsigned char const*, graphite2::Face&, graphite2::passtype, graphite2::Error&) /home/user/code/graphite/src/Pass.cpp:260:45
    #4 0x7f9856bdaf7b in graphite2::Pass::readPass(unsigned char const*, unsigned long, unsigned long, graphite2::Face&, graphite2::passtype, unsigned int, graphite2::Error&) /home/user/code/graphite/src/Pass.cpp:202:14
    #5 0x7f9856c50b33 in graphite2::Silf::readGraphite(unsigned char const*, unsigned long, graphite2::Face&, unsigned int) /home/user/code/graphite/src/Silf.cpp:216:14
    #6 0x7f9856b52831 in graphite2::Face::readGraphite(graphite2::Face::Table const&) /home/user/code/graphite/src/Face.cpp:149:14
    #7 0x7f9856a68bdb in (anonymous namespace)::load_face(graphite2::Face&, unsigned int) /home/user/code/graphite/src/gr_face.cpp:59:42
    #8 0x7f9856a68339 in gr_make_face_with_ops /home/user/code/graphite/src/gr_face.cpp:89:16
    #9 0x7f9856a6bfc3 in gr_make_file_face /home/user/code/graphite/src/gr_face.cpp:242:23
    #10 0x4f5f9d in Parameters::testFileFont() const /home/user/code/graphite/gr2fonttest/gr2FontTest.cpp:633:20
    #11 0x4fd33b in main /home/user/code/graphite/gr2fonttest/gr2FontTest.cpp:787:9
    #12 0x7f98565c1ec4 in __libc_start_main /build/eglibc-3GlaMS/eglibc-2.19/csu/libc-start.c:287
    #13 0x41b985 in _start (/home/user/Desktop/graphite/gr2fonttest+0x41b985)

Comment 1

[martin_hosken]

fixed? in or before 3d80c6b69fd647fdb134987e2e87ea830e93e3a4. Probably the same as some other bug.

Comment 2

[martin_hosken]

Sorry missed the UBSan aspect to this bug. This is a won't fix. The very next thing after assigning the value is we check it.

Comment 3

[Jonathan Kew [:jfkthame]]

I think the point, though, is that the behavior of the assignment is unspecified, because the value of the byte is outside the enum's range. So by the time you check it, you're already into uncharted territory.

The fix is to use a `byte` variable to fetch and validate the opcode, and only assign it to a typed `opcode` once you know it is within range.

Comment 4

[Jonathan Kew [:jfkthame]]

I just proposed https://github.com/silnrsi/graphite/pull/2, which I believe should fix this.

Comment 5

[martin_hosken]

PR merged (with slight modification)

Comment 6

[Tyson Smith [:tsmith]]

Verified with graphite revision 56671221b974024dd96cc9c6f592678ee6d24841

Comment 7

[Daniel Veditz [:dveditz]]

Graphite2 has been updated to 1.3.8 on all the relevant branches including ESRs