Skip to content

Commit 6a68841

Browse files
committed
[llvm][mustache] Fix UB in ASTNode::render()
The current implementation set a reference to a nullptr, leading to all kinds of problems. Instead, we can check the various uses to ensure we don't deref invalid memory, and improve the logic for how contexts are passed to children, since that was also subtly wrong in some cases.
1 parent 64e9a3f commit 6a68841

File tree

1 file changed

+33
-22
lines changed

1 file changed

+33
-22
lines changed

llvm/lib/Support/Mustache.cpp

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88
#include "llvm/Support/Mustache.h"
99
#include "llvm/ADT/SmallVector.h"
10+
#include "llvm/IR/Value.h"
1011
#include "llvm/Support/Error.h"
1112
#include "llvm/Support/raw_ostream.h"
1213
#include <sstream>
@@ -23,6 +24,13 @@ static bool isFalsey(const json::Value &V) {
2324
(V.getAsArray() && V.getAsArray()->empty());
2425
}
2526

27+
static bool isContextFalsey(const json::Value *V) {
28+
// A missing context (represented by a nullptr) is defined as falsey.
29+
if (!V)
30+
return true;
31+
return isFalsey(*V);
32+
}
33+
2634
static Accessor splitMustacheString(StringRef Str) {
2735
// We split the mustache string into an accessor.
2836
// For example:
@@ -560,68 +568,71 @@ void toMustacheString(const json::Value &Data, raw_ostream &OS) {
560568
}
561569
}
562570

563-
void ASTNode::render(const json::Value &Data, raw_ostream &OS) {
564-
ParentContext = &Data;
571+
void ASTNode::render(const json::Value &CurrentCtx, raw_ostream &OS) {
572+
// Set the parent context to the incoming context so that we
573+
// can walk up the context tree correctly in findContext().
574+
ParentContext = &CurrentCtx;
565575
const json::Value *ContextPtr = Ty == Root ? ParentContext : findContext();
566-
const json::Value &Context = ContextPtr ? *ContextPtr : nullptr;
567576

568577
switch (Ty) {
569578
case Root:
570-
renderChild(Data, OS);
579+
renderChild(CurrentCtx, OS);
571580
return;
572581
case Text:
573582
OS << Body;
574583
return;
575584
case Partial: {
576585
auto Partial = Partials.find(AccessorValue[0]);
577586
if (Partial != Partials.end())
578-
renderPartial(Data, OS, Partial->getValue().get());
587+
renderPartial(CurrentCtx, OS, Partial->getValue().get());
579588
return;
580589
}
581590
case Variable: {
582591
auto Lambda = Lambdas.find(AccessorValue[0]);
583-
if (Lambda != Lambdas.end())
584-
renderLambdas(Data, OS, Lambda->getValue());
585-
else {
592+
if (Lambda != Lambdas.end()) {
593+
renderLambdas(CurrentCtx, OS, Lambda->getValue());
594+
} else if (ContextPtr) {
586595
EscapeStringStream ES(OS, Escapes);
587-
toMustacheString(Context, ES);
596+
toMustacheString(*ContextPtr, ES);
588597
}
589598
return;
590599
}
591600
case UnescapeVariable: {
592601
auto Lambda = Lambdas.find(AccessorValue[0]);
593-
if (Lambda != Lambdas.end())
594-
renderLambdas(Data, OS, Lambda->getValue());
595-
else
596-
toMustacheString(Context, OS);
602+
if (Lambda != Lambdas.end()) {
603+
renderLambdas(CurrentCtx, OS, Lambda->getValue());
604+
} else if (ContextPtr) {
605+
toMustacheString(*ContextPtr, OS);
606+
}
597607
return;
598608
}
599609
case Section: {
600-
// Sections are not rendered if the context is falsey.
601610
auto SectionLambda = SectionLambdas.find(AccessorValue[0]);
602611
bool IsLambda = SectionLambda != SectionLambdas.end();
603-
if (isFalsey(Context) && !IsLambda)
612+
613+
if (isContextFalsey(ContextPtr) && !IsLambda)
604614
return;
605615

606616
if (IsLambda) {
607-
renderSectionLambdas(Data, OS, SectionLambda->getValue());
617+
renderSectionLambdas(CurrentCtx, OS, SectionLambda->getValue());
608618
return;
609619
}
610620

611-
if (Context.getAsArray()) {
612-
const json::Array *Arr = Context.getAsArray();
621+
if (const json::Array *Arr = ContextPtr->getAsArray()) {
613622
for (const json::Value &V : *Arr)
614623
renderChild(V, OS);
615624
return;
616625
}
617-
renderChild(Context, OS);
626+
renderChild(*ContextPtr, OS);
618627
return;
619628
}
620629
case InvertSection: {
621630
bool IsLambda = SectionLambdas.contains(AccessorValue[0]);
622-
if (!isFalsey(Context) || IsLambda)
623-
return;
624-
renderChild(Context, OS);
631+
if (isContextFalsey(ContextPtr) && !IsLambda) {
632+
// The context for the children remains UNCHANGED from the parent's.
633+
// We pass 'Data', which is this node's original incoming context.
634+
renderChild(CurrentCtx, OS);
635+
}
625636
return;
626637
}
627638
}

0 commit comments

Comments
 (0)