Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Out of Memory (crash) on infinite loop #2781

Closed
zyingp opened this issue Dec 3, 2018 · 13 comments
Closed

Out of Memory (crash) on infinite loop #2781

zyingp opened this issue Dec 3, 2018 · 13 comments

Comments

@zyingp
Copy link

zyingp commented Dec 3, 2018

Some malformed input could cause the libsass/sassc program out of memory and crashes. These malformed files are not big and are simply adding an extra '/' or '&' in original files (created by fuzzing). The problem can be reproduced both in version 3.5.5 and the master branch (accessed on 2018/12/2) code.

Sample malformed input files include https://github.com/zyingp/temp/blob/master/sass_mem_37, and https://github.com/zyingp/temp/blob/master/sass_mem_38 .

Run
./sassc sass_mem_37

sass_mem_37

For example, sass_mem_37 looks like below, where the '&' symbol before 1 is extra:

$i: 1;

.foo {
  @while $i != 5 {
    a: $i;
    $i: $i +&1;
  }
}

(Similar, the sass_mem_38 file, the '/' is added later https://github.com/zyingp/temp/blob/master/sass_mem_38)

Call stack

Since the problem will crash so when I randomly stop the problem, the call stacks looks like below:

#0	0x00000001003b8201 in Sass::Offset::add(char const*, char const*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/position.cpp:52
#1	0x00000001003b71bf in Sass::Offset::inc(char const*, char const*) const at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/position.cpp:75
#2	0x00000001003b7a2b in Sass::Offset::Offset(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/position.cpp:21
#3	0x00000001003b7b7d in Sass::Offset::Offset(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/position.cpp:20
#4	0x000000010072c040 in Sass::Emitter::append_string(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/emitter.cpp:148
#5	0x000000010072dd6c in Sass::Emitter::append_token(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, Sass::AST_Node*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/emitter.cpp:174
#6	0x0000000100707fe9 in Sass::Inspect::operator()(Sass::String_Quoted*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/inspect.cpp:728
#7	0x0000000100076a77 in Sass::String_Quoted::perform(Sass::Operation<void>*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/ast_values.hpp:391
#8	0x00000001006f5fd1 in Sass::Inspect::operator()(Sass::List*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/inspect.cpp:430
#9	0x00000001000724d7 in Sass::List::perform(Sass::Operation<void>*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/ast_values.hpp:86
#10	0x0000000100003dea in Sass::AST_Node::to_string(Sass_Inspect_Options) const at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/ast.cpp:37
#11	0x00000001007ebc83 in Sass::Operators::op_strings(Sass::Operand, Sass::Value&, Sass::Value&, Sass_Inspect_Options, Sass::ParserState const&, bool) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/operators.cpp:81
#12	0x00000001004f7c55 in Sass::Eval::operator()(Sass::Binary_Expression*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/eval.cpp:869
#13	0x00000001000735b7 in Sass::Binary_Expression::perform(Sass::Operation<void>*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/ast_values.hpp:137
#14	0x00000001004e7660 in Sass::Eval::operator()(Sass::List*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/eval.cpp:522
#15	0x00000001000724d7 in Sass::List::perform(Sass::Operation<void>*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/ast_values.hpp:86
#16	0x0000000100561c24 in Sass::Expand::operator()(Sass::Assignment*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/expand.cpp:322
#17	0x000000010002ac84 in Sass::Assignment::perform(Sass::Operation<void>*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/ast.hpp:585
#18	0x000000010055405d in Sass::Expand::append_block(Sass::Block*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/expand.cpp:807
#19	0x0000000100574104 in Sass::Expand::operator()(Sass::While*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/expand.cpp:575
#20	0x000000010002bd17 in Sass::While::perform(Sass::Operation<void>*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/ast.hpp:710
#21	0x000000010055405d in Sass::Expand::append_block(Sass::Block*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/expand.cpp:807
#22	0x0000000100552951 in Sass::Expand::operator()(Sass::Block*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/expand.cpp:72
#23	0x0000000100558a3a in Sass::Expand::operator()(Sass::Ruleset*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/expand.cpp:144
#24	0x000000010002a124 in Sass::Ruleset::perform(Sass::Operation<void>*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/ast.hpp:487
#25	0x000000010055405d in Sass::Expand::append_block(Sass::Block*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/expand.cpp:807
#26	0x0000000100552951 in Sass::Expand::operator()(Sass::Block*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/expand.cpp:72
#27	0x0000000100173194 in Sass::Context::compile() at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/context.cpp:678
#28	0x000000010016f9bc in Sass::File_Context::parse() at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/context.cpp:605
#29	0x000000010077a401 in Sass::sass_parse_block(Sass_Compiler*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/sass_context.cpp:234
#30	0x0000000100779b8b in ::sass_compiler_parse(Sass_Compiler *) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/sass_context.cpp:483
#31	0x00000001007792fa in sass_compile_context(Sass_Context*, Sass::Context*) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/sass_context.cpp:371
#32	0x000000010077971e in ::sass_compile_file_context(Sass_File_Context *) at /Users/zengyingpei/Downloads/fuzzer/sass/libsass/src/sass_context.cpp:470
#33	0x0000000100001797 in compile_file at /Users/zengyingpei/Downloads/fuzzer/sass/sassc/sassc.c:158
#34	0x00000001000020d7 in main at /Users/zengyingpei/Downloads/fuzzer/sass/sassc/sassc.c:370
#35	0x00007fff701cb015 in start ()

Where in Sass::Inspect::operator()(Sass::String_Quoted*) at inspect.cpp:728, the append_token(s->value(), s); line the s becomes a very long string (1.foo 1.foo 1.foo 1.foo 1.foo...) and is keeping increasing. Seems the problem enters some endless loop. The memory footprint of the program grows quickly (to 1GB and above) and later the problem is killed by the system.

@glebm
Copy link
Contributor

glebm commented Dec 3, 2018

The endless loop here is while $i != 5.

If you add @debug($i); there, it becomes clear what's going on:

oom.scss:7 DEBUG: 1.foo 1.foo 1.foo 1.foo 1.foo 1.foo 1.foo 1.foo ...

Ruby Sass and Dart Sass do the same thing here.

Intended behaviour, I guess.

@nex3, Should number + selector be a valid operation in Sass? It is now but it seems weird that this is allowed.

@nex3
Copy link
Contributor

nex3 commented Dec 3, 2018

This is expected; the + operation by default converts its arguments to strings and concatenates them. Whether it should be so permissive is an open question, but even if not I don't think it's worth breaking backwards-compatibility to fix.

@xzyfer
Copy link
Contributor

xzyfer commented Dec 4, 2018 via email

@zyingp
Copy link
Author

zyingp commented Dec 6, 2018

The original file has no problem and only a simple extra symbol like '/' or '&' crashes the sassc program. I guess that sometime such extra symbol may introduced in by typos. I am not sure whether there could be some governors in libsass (like an upper bound on loop)? When we are writing code (C/C++/Java), we know the compilers will not crash on our typos or small errors. I am not sure whether sass file is some kind of "code", and sassc could show equivalent robustness.

@nex3
Copy link
Contributor

nex3 commented Dec 6, 2018

@zyingp Sass functions more like a programming language interpreter than a compiler, in the sense that it directly runs users' programs rather than compiling them to an intermediate format before running them. A more direct comparison would be to what a C, C++, or Java program would do if you ran a loop that continually allocated more and more memory. For C and C++, it would certainly crash in the same way LibSass is doing here. Java is more sophisticated about its memory management and would probably throw a more graceful error once its memory ceiling was reached, but it would still produce an error of some kind.

@mgreter
Copy link
Contributor

mgreter commented Dec 14, 2018

I agree with what @nex3 said, sass functions are closer to real programming logic, so it is indeed possible that any input file creates endless loops, thus consuming memory ad infinite. Unfortunately in c++ this means it will segfault at some point due to exhausting the stack (depending on the underlying stack memory limitations). IMO we can't catch this type of errors in a portable way (heap out of memory should already be catched though). Adding a counter and enforcing an upper limit may work for most scenarios, but it does only hide the underlying issue, while crippling the maximum on most envs and also has a performance impact. IMO this should be closed as "will not fix" due to "as designed, user input issue".

On a side note: these kind of issues seem more important for eg. sassmeister or libsass srcmap inspector, since it allows some kind of DOS for remotely available system. But we always had this issue, and I personally use GRSEC/SELINUX to circumvent it.

@xzyfer
Copy link
Contributor

xzyfer commented Dec 17, 2018

On a side note: these kind of issues seem more important for eg. sassmeister or libsass srcmap inspector

I know some services have a hard time out on compilations to mitigate DOS attacks from this vector.

@xzyfer
Copy link
Contributor

xzyfer commented Dec 17, 2018

I agree the comments aboves. This is essentially by design.

@xzyfer xzyfer closed this as completed Dec 17, 2018
@rjoshi18
Copy link

@xzyfer Could you please tell, How this issue has fixed here?
Thanks

@glebm
Copy link
Contributor

glebm commented Dec 26, 2018

@rjoshi18 This issue is closed as "won't fix" / "works as intended" ("by design").

@glebm glebm added the Fuzzy label Apr 15, 2019
@glebm glebm changed the title Out of Memory (crash) with malformed input files (Adding an extra '/' or '&') Out of Memory (crash) on infinite loop Apr 15, 2019
@xi
Copy link

xi commented Jun 5, 2019

Assigned CVE-2018-19826

@nschonni
Copy link
Collaborator

nschonni commented Jun 5, 2019

Why assign a CVE to something that wasn't accepted as a bug/issue
Edit: I see it's marked as "disputed"

@xi
Copy link

xi commented Jun 5, 2019

I am not assigning these. I am just adding the references so that they are easier to find.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants