From 4039b0ef1ed3d3fd7010a551b119b3fa5bc1748c Mon Sep 17 00:00:00 2001 From: Abhishek1152 Date: Thu, 26 Mar 2026 19:07:49 +0530 Subject: [PATCH] Confidential checker toString and generics fixes --- .../confidential/qual/NonConfidential.java | 2 +- .../ConfidentialAnnotatedTypeFactory.java | 67 +------------ .../confidential/ConfidentialArithmetic.java | 58 +++++++++++ .../confidential/ConfidentialAssignment.java | 69 +++++++++++++ .../confidential/ConfidentialCasting.java | 38 ++++++++ .../confidential/ConfidentialControlFlow.java | 42 ++++++++ .../confidential/ConfidentialGenerics.java | 62 ++++++++++++ .../ConfidentialHardCodeLiteral.java | 12 +++ .../confidential/ConfidentialMethodArgs.java | 96 +++++++++++++++++++ .../confidential/ConfidentialToString.java | 6 ++ 10 files changed, 387 insertions(+), 65 deletions(-) create mode 100644 checker/tests/confidential/ConfidentialArithmetic.java create mode 100644 checker/tests/confidential/ConfidentialAssignment.java create mode 100644 checker/tests/confidential/ConfidentialCasting.java create mode 100644 checker/tests/confidential/ConfidentialControlFlow.java create mode 100644 checker/tests/confidential/ConfidentialGenerics.java create mode 100644 checker/tests/confidential/ConfidentialHardCodeLiteral.java create mode 100644 checker/tests/confidential/ConfidentialMethodArgs.java diff --git a/checker-qual/src/main/java/org/checkerframework/checker/confidential/qual/NonConfidential.java b/checker-qual/src/main/java/org/checkerframework/checker/confidential/qual/NonConfidential.java index 76f9997cc96a..5e9f7ae138bd 100644 --- a/checker-qual/src/main/java/org/checkerframework/checker/confidential/qual/NonConfidential.java +++ b/checker-qual/src/main/java/org/checkerframework/checker/confidential/qual/NonConfidential.java @@ -25,5 +25,5 @@ @SubtypeOf(UnknownConfidential.class) @QualifierForLiterals({LiteralKind.STRING, LiteralKind.PRIMITIVE}) @DefaultQualifierInHierarchy -@DefaultFor(value = {TypeUseLocation.LOCAL_VARIABLE, TypeUseLocation.UPPER_BOUND}) +@DefaultFor(value = {TypeUseLocation.LOCAL_VARIABLE}) public @interface NonConfidential {} diff --git a/checker/src/main/java/org/checkerframework/checker/confidential/ConfidentialAnnotatedTypeFactory.java b/checker/src/main/java/org/checkerframework/checker/confidential/ConfidentialAnnotatedTypeFactory.java index 64bf3c4f7c04..2ebf0c3b042a 100644 --- a/checker/src/main/java/org/checkerframework/checker/confidential/ConfidentialAnnotatedTypeFactory.java +++ b/checker/src/main/java/org/checkerframework/checker/confidential/ConfidentialAnnotatedTypeFactory.java @@ -1,9 +1,7 @@ package org.checkerframework.checker.confidential; -import com.sun.source.tree.MethodInvocationTree; import java.util.Set; import javax.lang.model.element.AnnotationMirror; -import javax.lang.model.element.ExecutableElement; import org.checkerframework.checker.confidential.qual.BottomConfidential; import org.checkerframework.checker.confidential.qual.Confidential; import org.checkerframework.checker.confidential.qual.NonConfidential; @@ -14,13 +12,8 @@ import org.checkerframework.framework.flow.CFStore; import org.checkerframework.framework.flow.CFTransfer; import org.checkerframework.framework.flow.CFValue; -import org.checkerframework.framework.type.AnnotatedTypeFactory; -import org.checkerframework.framework.type.AnnotatedTypeMirror; -import org.checkerframework.framework.type.treeannotator.ListTreeAnnotator; -import org.checkerframework.framework.type.treeannotator.TreeAnnotator; import org.checkerframework.javacutil.AnnotationBuilder; import org.checkerframework.javacutil.AnnotationMirrorSet; -import org.checkerframework.javacutil.TreeUtils; /** Annotated type factory for the Confidential Checker. */ public class ConfidentialAnnotatedTypeFactory extends BaseAnnotatedTypeFactory { @@ -56,10 +49,6 @@ public class ConfidentialAnnotatedTypeFactory extends BaseAnnotatedTypeFactory { /** A singleton set containing the {@code @}{@link NonConfidential} annotation mirror. */ private final AnnotationMirrorSet setOfNonConfidential; - /** The Object.toString method. */ - private final ExecutableElement objectToString = - TreeUtils.getMethod("java.lang.Object", "toString", 0, processingEnv); - /** * Creates a {@link ConfidentialAnnotatedTypeFactory}. * @@ -78,64 +67,14 @@ public ConfidentialAnnotatedTypeFactory(BaseTypeChecker checker) { postInit(); } - @Override - protected Set getEnumConstructorQualifiers() { - return setOfNonConfidential; - } - - @Override - public TreeAnnotator createTreeAnnotator() { - return new ListTreeAnnotator( - super.createTreeAnnotator(), - new ConfidentialAnnotatedTypeFactory.ConfidentialTreeAnnotator(this)); - } - @Override public CFTransfer createFlowTransferFunction( CFAbstractAnalysis analysis) { return new ConfidentialTransfer(analysis); } - /** - * A TreeAnnotator to enforce certain toString return type rules: - * - *
    - *
  • toString(@NonConfidential this) can return @NonConfidential or @Confidential String - *
  • toString(@Confidential this) can return @Confidential String - *
- */ - private class ConfidentialTreeAnnotator extends TreeAnnotator { - /** - * Creates a {@link ConfidentialAnnotatedTypeFactory.ConfidentialTreeAnnotator} - * - * @param atypeFactory the annotated type factory - */ - public ConfidentialTreeAnnotator(AnnotatedTypeFactory atypeFactory) { - super(atypeFactory); - } - - /** - * Visits a method invocation node. Enforces specific type-checking rules for Object.toString() - * that allow a @NonConfidential Object to return a @NonConfidential String. - * - *

Supplements the @Confidential String return in Object.toString() to cover all secure use - * cases, i.e. all cases covered by a @PolyConfidential receiver and return excepting - * a @NonConfidential String from @Confidential receivers. - * - * @param tree an AST node representing a method call - * @param type the type obtained from tree - */ - @Override - public Void visitMethodInvocation(MethodInvocationTree tree, AnnotatedTypeMirror type) { - if (TreeUtils.isMethodInvocation(tree, objectToString, processingEnv)) { - AnnotatedTypeMirror receiver = getReceiverType(tree); - if (receiver.hasPrimaryAnnotation(NONCONFIDENTIAL)) { - type.replaceAnnotation(NONCONFIDENTIAL); - } else { - type.replaceAnnotation(CONFIDENTIAL); - } - } - return super.visitMethodInvocation(tree, type); - } + @Override + protected Set getEnumConstructorQualifiers() { + return setOfNonConfidential; } } diff --git a/checker/tests/confidential/ConfidentialArithmetic.java b/checker/tests/confidential/ConfidentialArithmetic.java new file mode 100644 index 000000000000..ad007508c1f0 --- /dev/null +++ b/checker/tests/confidential/ConfidentialArithmetic.java @@ -0,0 +1,58 @@ +import org.checkerframework.checker.confidential.qual.Confidential; +import org.checkerframework.checker.confidential.qual.NonConfidential; + +public class ConfidentialArithmetic { + void addition(@Confidential int confInt, @NonConfidential int nonConfInt) { + @Confidential int r1 = confInt + nonConfInt; + @Confidential int r2 = confInt + confInt; + @NonConfidential int r3 = nonConfInt + nonConfInt; + + // :: error: [assignment] + @NonConfidential int r4 = nonConfInt + confInt; + // :: error: [assignment] + @NonConfidential int r5 = confInt + nonConfInt; + // :: error: [assignment] + @NonConfidential int r6 = confInt + confInt; + } + + void subtraction(@Confidential int confInt, @NonConfidential int nonConfInt) { + @Confidential int r1 = confInt - nonConfInt; + @NonConfidential int r2 = nonConfInt - nonConfInt; + + // :: error: [assignment] + @NonConfidential int r3 = confInt - confInt; + // :: error: [assignment] + @NonConfidential int r4 = confInt - nonConfInt; + // :: error: [assignment] + @NonConfidential int r5 = nonConfInt - confInt; + } + + void compoundAssignment(@Confidential int confInt, @NonConfidential int nonConfInt) { + @NonConfidential int x = 0; + x += nonConfInt; + + @Confidential int y = 0; + y += confInt; + y += nonConfInt; + } + + void comparison(@Confidential int confInt, @NonConfidential int nonConfInt) { + // :: error: [assignment] + @NonConfidential boolean b1 = confInt > nonConfInt; + // :: error: [assignment] + @NonConfidential boolean b2 = confInt == nonConfInt; + @NonConfidential boolean b3 = nonConfInt < nonConfInt; + } + + void intStringConcatenation( + @Confidential int confInt, + @NonConfidential int nonConfInt, + @Confidential String confStr, + @NonConfidential String nonConfStr) { + @Confidential String s1 = confStr + confInt; + @NonConfidential String s2 = nonConfStr + nonConfInt; + + // :: error: [assignment] + @NonConfidential String s3 = nonConfStr + confInt; + } +} diff --git a/checker/tests/confidential/ConfidentialAssignment.java b/checker/tests/confidential/ConfidentialAssignment.java new file mode 100644 index 000000000000..6170f349063a --- /dev/null +++ b/checker/tests/confidential/ConfidentialAssignment.java @@ -0,0 +1,69 @@ +import org.checkerframework.checker.confidential.qual.Confidential; +import org.checkerframework.checker.confidential.qual.NonConfidential; + +public class ConfidentialAssignment { + void stringAssignments(@Confidential String conf, @NonConfidential String nonConf) { + @Confidential String s1 = nonConf; + @Confidential String s3 = conf; + @NonConfidential String s4 = nonConf; + + // :: error: [assignment] + @NonConfidential String s2 = conf; + } + + void literalDefaults() { + // Literals default to @NonConfidential + @NonConfidential String s = "hello"; + @Confidential String cs = "hello"; + } + + void intAssignments(@Confidential int confInt, @NonConfidential int nonConfInt) { + @Confidential int i1 = nonConfInt; + @Confidential int i3 = confInt; + @NonConfidential int i4 = nonConfInt; + + // :: error: [assignment] + @NonConfidential int i2 = confInt; + } + + void integerAssignments(@Confidential Integer confInt, @NonConfidential Integer nonConfInt) { + @Confidential Integer i1 = nonConfInt; + @Confidential Integer i3 = confInt; + @NonConfidential Integer i4 = nonConfInt; + + // :: error: [assignment] + @NonConfidential Integer i2 = confInt; + } + + void boxing(@Confidential int confInt, @NonConfidential int nonConfInt) { + @Confidential Integer boxed1 = confInt; + @NonConfidential Integer boxed2 = nonConfInt; + + // :: error: [assignment] + @NonConfidential Integer boxed3 = confInt; + } + + void unboxing(@Confidential Integer confInteger, @NonConfidential Integer nonConfInteger) { + @Confidential int unboxed1 = confInteger; + @NonConfidential int unboxed2 = nonConfInteger; + + // :: error: [assignment] + @NonConfidential int unboxed3 = confInteger; + } + + void intLiteralDefaults() { + @NonConfidential int i = 42; + @Confidential int ci = 42; + @NonConfidential Integer bi = 42; + @Confidential Integer cbi = 42; + } + + void reassignment(@Confidential String conf, @NonConfidential String nonConf) { + @Confidential String s = nonConf; + s = conf; + + @NonConfidential String s2 = nonConf; + // :: error: [assignment] + s2 = conf; + } +} diff --git a/checker/tests/confidential/ConfidentialCasting.java b/checker/tests/confidential/ConfidentialCasting.java new file mode 100644 index 000000000000..9c72463fc49a --- /dev/null +++ b/checker/tests/confidential/ConfidentialCasting.java @@ -0,0 +1,38 @@ +import org.checkerframework.checker.confidential.qual.Confidential; +import org.checkerframework.checker.confidential.qual.NonConfidential; + +public class ConfidentialCasting { + void castToConfidential(@NonConfidential String nonConf, @NonConfidential int nonConfInt) { + // :: warning: [cast.unsafe] + @Confidential String s = (@Confidential String) nonConf; + // :: warning: [cast.unsafe] + @Confidential int i = (@Confidential int) nonConfInt; + } + + void castConfidentialToConfidential(@Confidential String conf) { + @Confidential String s = (@Confidential String) conf; + } + + void castToNonConfidential(@Confidential String conf, @Confidential int confInt) { + // :: error: [assignment] :: warning: [cast.unsafe] + @NonConfidential String s = (@NonConfidential String) conf; + // :: error: [assignment] :: warning: [cast.unsafe] + @NonConfidential int i = (@NonConfidential int) confInt; + } + + void numericWidening(@Confidential int confInt, @NonConfidential int nonConfInt) { + @Confidential long cl = confInt; + @NonConfidential long ncl = nonConfInt; + + // :: error: [assignment] + @NonConfidential long ncl2 = confInt; + } + + void numericNarrowing(@Confidential long confLong, @NonConfidential long nonConfLong) { + @Confidential int ci = (int) confLong; + @NonConfidential int nci = (int) nonConfLong; + + // :: error: [assignment] + @NonConfidential int nci2 = (int) confLong; + } +} diff --git a/checker/tests/confidential/ConfidentialControlFlow.java b/checker/tests/confidential/ConfidentialControlFlow.java new file mode 100644 index 000000000000..6a96a265b92a --- /dev/null +++ b/checker/tests/confidential/ConfidentialControlFlow.java @@ -0,0 +1,42 @@ +import org.checkerframework.checker.confidential.qual.Confidential; +import org.checkerframework.checker.confidential.qual.NonConfidential; + +public class ConfidentialControlFlow { + void ternary(boolean flag, @Confidential String conf, @NonConfidential String nonConf) { + @NonConfidential String s1 = flag ? nonConf : nonConf; + @Confidential String s2 = flag ? conf : conf; + @Confidential String s3 = flag ? conf : nonConf; + @Confidential String s4 = flag ? nonConf : conf; + + // :: error: [assignment] + @NonConfidential String s5 = flag ? conf : nonConf; + // :: error: [assignment] + @NonConfidential String s6 = flag ? nonConf : conf; + } + + void ternaryInt(boolean flag, @Confidential int conf, @NonConfidential int nonConf) { + @NonConfidential int i1 = flag ? nonConf : nonConf; + @Confidential int i2 = flag ? conf : nonConf; + + // :: error: [assignment] + @NonConfidential int i3 = flag ? conf : nonConf; + } + + void ifElseFlow(boolean flag, @Confidential String conf, @NonConfidential String nonConf) { + @Confidential String confRes; + if (flag) { + confRes = conf; + } else { + confRes = nonConf; + } + } + + void ternaryAsArg(boolean flag, @Confidential String conf, @NonConfidential String nonConf) { + sinkNonConf(flag ? nonConf : nonConf); + + // :: error: [argument] + sinkNonConf(flag ? conf : nonConf); + } + + void sinkNonConf(@NonConfidential String nonConf) {} +} diff --git a/checker/tests/confidential/ConfidentialGenerics.java b/checker/tests/confidential/ConfidentialGenerics.java new file mode 100644 index 000000000000..847ed6b3fee3 --- /dev/null +++ b/checker/tests/confidential/ConfidentialGenerics.java @@ -0,0 +1,62 @@ +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.checkerframework.checker.confidential.qual.Confidential; +import org.checkerframework.checker.confidential.qual.NonConfidential; + +public class ConfidentialGenerics { + void testConfidentialMap() { + Map map = new HashMap<>(); + @Confidential String secret = "password"; + map.put("key", secret); + + @Confidential String retrieved = map.get("key"); + + // :: error: [assignment] + @NonConfidential String leaked = map.get("key"); + } + + void testNonConfidentialMap() { + Map map = new HashMap<>(); + @Confidential String secret = "password"; + + // :: error: [argument] + map.put("key", secret); + + @NonConfidential String safe = map.get("key"); + } + + void testConfidentialSet() { + Set<@Confidential String> set = new HashSet<>(); + @Confidential String secret = "token"; + set.add(secret); + } + + void testNonConfidentialSet() { + Set set = new HashSet<>(); + @Confidential String secret = "token"; + + // :: error: [argument] + set.add(secret); + } + + void testConfidentialList() { + List<@Confidential String> list = new ArrayList<>(); + @Confidential String secret = "ssn"; + list.add(secret); + + @Confidential String retrieved = list.get(0); + + // :: error: [assignment] + @NonConfidential String leaked = list.get(0); + } + + void testConfidentialKeys() { + Map<@Confidential String, String> map = new HashMap<>(); + @Confidential String confKey = "secret-key"; + map.put(confKey, "value"); + } +} diff --git a/checker/tests/confidential/ConfidentialHardCodeLiteral.java b/checker/tests/confidential/ConfidentialHardCodeLiteral.java new file mode 100644 index 000000000000..1db35a210f62 --- /dev/null +++ b/checker/tests/confidential/ConfidentialHardCodeLiteral.java @@ -0,0 +1,12 @@ +package confidential; + +import org.checkerframework.checker.confidential.qual.Confidential; + +public class ConfidentialHardCodeLiteral { + void hardCodedLiteral() { + // This should fail to prevent leak in the VCS + @Confidential String secret = "secret_key"; + @Confidential int pan = 123456; + @Confidential Integer dob = 123456; + } +} diff --git a/checker/tests/confidential/ConfidentialMethodArgs.java b/checker/tests/confidential/ConfidentialMethodArgs.java new file mode 100644 index 000000000000..8bcb68e067c5 --- /dev/null +++ b/checker/tests/confidential/ConfidentialMethodArgs.java @@ -0,0 +1,96 @@ +import org.checkerframework.checker.confidential.qual.Confidential; +import org.checkerframework.checker.confidential.qual.NonConfidential; + +public class ConfidentialMethodArgs { + void testStringArgs(@Confidential String conf, @NonConfidential String nonConf) { + sinkNonConf(nonConf); + sinkConf(nonConf); + sinkConf(conf); + + // :: error: [argument] + sinkNonConf(conf); + } + + void sinkNonConfInt(@NonConfidential int i) {} + + void sinkConfInt(@Confidential int i) {} + + void sinkNonConfInteger(@NonConfidential Integer i) {} + + void sinkConfInteger(@Confidential Integer i) {} + + void testIntArgs(@Confidential int confInt, @NonConfidential int nonConfInt) { + sinkNonConfInt(nonConfInt); + sinkConfInt(nonConfInt); + sinkConfInt(confInt); + + // :: error: [argument] + sinkNonConfInt(confInt); + } + + void testIntegerArgs(@Confidential Integer confInt, @NonConfidential Integer nonConfInt) { + sinkNonConfInteger(nonConfInt); + sinkConfInteger(nonConfInt); + sinkConfInteger(confInt); + + // :: error: [argument] + sinkNonConfInteger(confInt); + } + + void testBoxingArgs(@Confidential int confInt, @NonConfidential int nonConfInt) { + sinkConfInteger(confInt); + sinkNonConfInteger(nonConfInt); + + // :: error: [argument] + sinkNonConfInteger(confInt); + } + + void testLiteralArgs() { + sinkNonConf("hello"); + sinkConf("hello"); + sinkNonConfInt(42); + sinkConfInt(42); + } + + @NonConfidential + String returnNonConf(@NonConfidential String s) { + return s; + } + + @NonConfidential + String returnNonConfError(@Confidential String s) { + // :: error: [return] + return s; + } + + @Confidential + String returnConf(@Confidential String s) { + return s; + } + + @Confidential + String returnConfFromNonConf(@NonConfidential String s) { + return s; + } + + @NonConfidential + int returnNonConfInt(@NonConfidential int i) { + return i; + } + + @NonConfidential + int returnNonConfIntError(@Confidential int i) { + // :: error: [return] + return i; + } + + @Confidential + int returnConfInt(@Confidential int i) { + return i; + } + + void sinkNonConf(@NonConfidential String s) {} + + void sinkConf(@Confidential String s) {} +} + diff --git a/checker/tests/confidential/ConfidentialToString.java b/checker/tests/confidential/ConfidentialToString.java index 96d178a9c14e..e8973c014506 100644 --- a/checker/tests/confidential/ConfidentialToString.java +++ b/checker/tests/confidential/ConfidentialToString.java @@ -11,4 +11,10 @@ void confObj(@Confidential Object confObj) { void nonConfObj(@NonConfidential Object nonConfObj) { @NonConfidential String nonConfRes = nonConfObj.toString(); } + + void confStr(@Confidential String confStr) { + // :: error: [assignment] + @NonConfidential String nonConfStrRes = confStr.toString(); + @Confidential String confStrRes = confStr.toString(); + } }