Skip to content

Java: Add MaDs for java.crypto.KDF #20345

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Java: Add MaDs for java.crypto.KDF #20345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a34b362
Java: accept new test results after extractor update
IdrissRio Aug 19, 2025
f762cf1
Java: add dataflow test for newly added KDF API
IdrissRio Sep 2, 2025
dba12f3
Java: Add MaDs for `java.crypto.KDF`
IdrissRio Sep 2, 2025
ae786de
Java: accept new test results
IdrissRio Sep 2, 2025
ccfbf55
Java: Add new change note
IdrissRio Sep 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions java/ql/lib/change-notes/2025-09-02-kdf-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added taint flow model for `java.crypto.KDF`.
15 changes: 15 additions & 0 deletions java/ql/lib/ext/javax.crypto.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,21 @@ extensions:
- ["javax.crypto", "Cipher", True, "init", "(int,Key,AlgorithmParameterSpec,SecureRandom)", "", "Argument[2]", "encryption-iv", "manual"]
- ["javax.crypto", "Cipher", False, "unwrap", "(byte[],String,int)", "", "Argument[0]", "credentials-key", "hq-generated"]
- ["javax.crypto", "CipherSpi", True, "engineUnwrap", "(byte[],String,int)", "", "Argument[0]", "credentials-key", "hq-generated"]
- addsTo:
pack: codeql/java-all
extensible: summaryModel
data:
- ["javax.crypto", "KDF", False, "getInstance", "(String)", "", "Argument[0]", "ReturnValue.SyntheticField[javax.crypto.KDF.algorithm]", "value", "manual"]
- ["javax.crypto", "KDF", False, "getInstance", "(String,Provider)", "", "Argument[0]", "ReturnValue.SyntheticField[javax.crypto.KDF.algorithm]", "value", "manual"]
- ["javax.crypto", "KDF", False, "getInstance", "(String,String)", "", "Argument[0]", "ReturnValue.SyntheticField[javax.crypto.KDF.algorithm]", "value", "manual"]
- ["javax.crypto", "KDF", False, "getInstance", "(String,KDFParameters)", "", "Argument[0]", "ReturnValue.SyntheticField[javax.crypto.KDF.algorithm]", "value", "manual"]
- ["javax.crypto", "KDF", False, "getInstance", "(String,KDFParameters,Provider)", "", "Argument[0]", "ReturnValue.SyntheticField[javax.crypto.KDF.algorithm]", "value", "manual"]
- ["javax.crypto", "KDF", False, "getInstance", "(String,KDFParameters,String)", "", "Argument[0]", "ReturnValue.SyntheticField[javax.crypto.KDF.algorithm]", "value", "manual"]
- ["javax.crypto", "KDF", True, "getAlgorithm", "()", "", "Argument[this].SyntheticField[javax.crypto.KDF.algorithm]", "ReturnValue", "value", "manual"]
- ["javax.crypto", "KDF", True, "getProvider", "()", "", "Argument[this]", "ReturnValue", "value", "manual"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line looks suspicious. It suggests that getProvider() is implemented as return this. Should this have been a taint model rather than a value-preserving step?

- ["javax.crypto", "KDF", True, "deriveKey", "(String,AlgorithmParameterSpec)", "", "Argument[1]", "ReturnValue", "taint", "manual"]
- ["javax.crypto", "KDF", True, "deriveData", "(AlgorithmParameterSpec)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["javax.crypto", "SecretKey", True, "getEncoded", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"]
- addsTo:
pack: codeql/java-all
extensible: neutralModel
Expand Down
18 changes: 18 additions & 0 deletions java/ql/lib/ext/javax.crypto.spec.model.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,24 @@ extensions:
- ["javax.crypto.spec", "GCMParameterSpec", True, "GCMParameterSpec", "", "", "Argument[1]", "Argument[this]", "taint", "manual"]
- ["javax.crypto.spec", "RC2ParameterSpec", True, "RC2ParameterSpec", "", "", "Argument[1]", "Argument[this]", "taint", "manual"]
- ["javax.crypto.spec", "RC5ParameterSpec", True, "RC5ParameterSpec", "", "", "Argument[3]", "Argument[this]", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[0]", "Argument[this]", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[this]", "ReturnValue", "taint", "manual"]
Comment on lines +10 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fluent method so the flow from this to return should be value. Once that's fixed, the two lines above will contain redundant information, as MaD has some support for fluent apis: You only need to provide the taint model that taints the argument - then the model that taints the output will be automatically added.

Suggested change
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[0]", "Argument[this]", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[this]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[0]", "Argument[this]", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(byte[])", "", "Argument[this]", "ReturnValue", "value", "manual"]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same applies to the other fluent api models below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll fix this 👍

- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(SecretKey)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(SecretKey)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addIKM", "(SecretKey)", "", "Argument[this]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addSalt", "(byte[])", "", "Argument[0]", "Argument[this]", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addSalt", "(byte[])", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addSalt", "(byte[])", "", "Argument[this]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addSalt", "(SecretKey)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addSalt", "(SecretKey)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "addSalt", "(SecretKey)", "", "Argument[this]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "thenExpand", "(byte[],int)", "", "Argument[this]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec$Builder", True, "thenExpand", "(byte[],int)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec", False, "expandOnly", "(SecretKey,byte[],int)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "HKDFParameterSpec", False, "expandOnly", "(SecretKey,byte[],int)", "", "Argument[1]", "ReturnValue", "taint", "manual"]
- ["javax.crypto.spec", "SecretKeySpec", False, "SecretKeySpec", "(byte[],String)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
- ["javax.crypto.spec", "SecretKeySpec", False, "SecretKeySpec", "(byte[],int,int,String)", "", "Argument[0]", "Argument[this]", "taint", "manual"]
- addsTo:
pack: codeql/java-all
extensible: sinkModel
Expand Down
120 changes: 38 additions & 82 deletions java/ql/test-kotlin1/library-tests/java-kotlin-collection-type-generic-methods/test.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,6 @@ methodWithDuplicate
| AbstractCollection<E> | removeAll | Collection<?> |
| AbstractCollection<E> | retainAll | Collection<?> |
| AbstractCollection<E> | toArray | T[] |
| AbstractCollection<Entry<K,V>> | add | Entry<K,V> |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff doesn't belong on this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, none of the Kotlin changes are part of this PR. I updated the base to idrissrio/java-upgrade-fix, so that commit now appears as well. Rebasing should remove it from the UI. You can safely ignore anything related to Kotlin.

| AbstractCollection<Entry<K,V>> | addAll | Collection<? extends Entry<K,V>> |
| AbstractCollection<Entry<K,V>> | contains | Object |
| AbstractCollection<Entry<K,V>> | containsAll | Collection<?> |
| AbstractCollection<Entry<K,V>> | remove | Object |
| AbstractCollection<Entry<K,V>> | removeAll | Collection<?> |
| AbstractCollection<Entry<K,V>> | retainAll | Collection<?> |
| AbstractCollection<Entry<K,V>> | toArray | T[] |
| AbstractCollection<K> | add | K |
| AbstractCollection<K> | addAll | Collection<? extends K> |
| AbstractCollection<K> | contains | Object |
| AbstractCollection<K> | containsAll | Collection<?> |
| AbstractCollection<K> | remove | Object |
| AbstractCollection<K> | removeAll | Collection<?> |
| AbstractCollection<K> | retainAll | Collection<?> |
| AbstractCollection<K> | toArray | T[] |
| AbstractCollection<Runnable> | add | Runnable |
| AbstractCollection<Runnable> | addAll | Collection<? extends Runnable> |
| AbstractCollection<Runnable> | contains | Object |
| AbstractCollection<Runnable> | containsAll | Collection<?> |
| AbstractCollection<Runnable> | remove | Object |
| AbstractCollection<Runnable> | removeAll | Collection<?> |
| AbstractCollection<Runnable> | retainAll | Collection<?> |
| AbstractCollection<Runnable> | toArray | T[] |
| AbstractCollection<String> | add | String |
| AbstractCollection<String> | addAll | Collection<? extends String> |
| AbstractCollection<String> | contains | Object |
Expand All @@ -56,14 +32,6 @@ methodWithDuplicate
| AbstractCollection<T> | removeAll | Collection<?> |
| AbstractCollection<T> | retainAll | Collection<?> |
| AbstractCollection<T> | toArray | T[] |
| AbstractCollection<V> | add | V |
| AbstractCollection<V> | addAll | Collection<? extends V> |
| AbstractCollection<V> | contains | Object |
| AbstractCollection<V> | containsAll | Collection<?> |
| AbstractCollection<V> | remove | Object |
| AbstractCollection<V> | removeAll | Collection<?> |
| AbstractCollection<V> | retainAll | Collection<?> |
| AbstractCollection<V> | toArray | T[] |
| AbstractList | add | E |
| AbstractList | add | int |
| AbstractList | addAll | Collection<? extends E> |
Expand Down Expand Up @@ -103,14 +71,14 @@ methodWithDuplicate
| AbstractMap | put | V |
| AbstractMap | putAll | Map<? extends K,? extends V> |
| AbstractMap | remove | Object |
| AbstractMap<Identity,Entry<?>> | containsKey | Object |
| AbstractMap<Identity,Entry<?>> | containsValue | Object |
| AbstractMap<Identity,Entry<?>> | equals | Object |
| AbstractMap<Identity,Entry<?>> | get | Object |
| AbstractMap<Identity,Entry<?>> | put | Entry<?> |
| AbstractMap<Identity,Entry<?>> | put | Identity |
| AbstractMap<Identity,Entry<?>> | putAll | Map<? extends Identity,? extends Entry<?>> |
| AbstractMap<Identity,Entry<?>> | remove | Object |
| AbstractMap<Identity,Object> | containsKey | Object |
| AbstractMap<Identity,Object> | containsValue | Object |
| AbstractMap<Identity,Object> | equals | Object |
| AbstractMap<Identity,Object> | get | Object |
| AbstractMap<Identity,Object> | put | Identity |
| AbstractMap<Identity,Object> | put | Object |
| AbstractMap<Identity,Object> | putAll | Map<? extends Identity,? extends Object> |
| AbstractMap<Identity,Object> | remove | Object |
| AbstractMap<K,V> | containsKey | Object |
| AbstractMap<K,V> | containsValue | Object |
| AbstractMap<K,V> | equals | Object |
Expand Down Expand Up @@ -179,17 +147,6 @@ methodWithDuplicate
| Collection<K> | retainAll | Collection<?> |
| Collection<K> | toArray | IntFunction<T[]> |
| Collection<K> | toArray | T[] |
| Collection<Runnable> | add | Runnable |
| Collection<Runnable> | addAll | Collection<? extends Runnable> |
| Collection<Runnable> | contains | Object |
| Collection<Runnable> | containsAll | Collection<?> |
| Collection<Runnable> | equals | Object |
| Collection<Runnable> | remove | Object |
| Collection<Runnable> | removeAll | Collection<?> |
| Collection<Runnable> | removeIf | Predicate<? super Runnable> |
| Collection<Runnable> | retainAll | Collection<?> |
| Collection<Runnable> | toArray | IntFunction<T[]> |
| Collection<Runnable> | toArray | T[] |
| Collection<String> | add | String |
| Collection<String> | addAll | Collection<? extends String> |
| Collection<String> | contains | Object |
Expand Down Expand Up @@ -332,37 +289,36 @@ methodWithDuplicate
| Map | replace | K |
| Map | replace | V |
| Map | replaceAll | BiFunction<? super K,? super V,? extends V> |
| Map<Identity,Entry<?>> | compute | BiFunction<? super Identity,? super Entry<?>,? extends Entry<?>> |
| Map<Identity,Entry<?>> | compute | Identity |
| Map<Identity,Entry<?>> | computeIfAbsent | Function<? super Identity,? extends Entry<?>> |
| Map<Identity,Entry<?>> | computeIfAbsent | Identity |
| Map<Identity,Entry<?>> | computeIfPresent | BiFunction<? super Identity,? super Entry<?>,? extends Entry<?>> |
| Map<Identity,Entry<?>> | computeIfPresent | Identity |
| Map<Identity,Entry<?>> | containsKey | Object |
| Map<Identity,Entry<?>> | containsValue | Object |
| Map<Identity,Entry<?>> | copyOf | Map<? extends K,? extends V> |
| Map<Identity,Entry<?>> | entry | K |
| Map<Identity,Entry<?>> | entry | V |
| Map<Identity,Entry<?>> | equals | Object |
| Map<Identity,Entry<?>> | forEach | BiConsumer<? super Identity,? super Entry<?>> |
| Map<Identity,Entry<?>> | get | Object |
| Map<Identity,Entry<?>> | getOrDefault | Entry<?> |
| Map<Identity,Entry<?>> | getOrDefault | Object |
| Map<Identity,Entry<?>> | merge | BiFunction<? super Entry<?>,? super Entry<?>,? extends Entry<?>> |
| Map<Identity,Entry<?>> | merge | Entry<?> |
| Map<Identity,Entry<?>> | merge | Identity |
| Map<Identity,Entry<?>> | of | K |
| Map<Identity,Entry<?>> | of | V |
| Map<Identity,Entry<?>> | ofEntries | Entry<? extends K,? extends V>[] |
| Map<Identity,Entry<?>> | put | Entry<?> |
| Map<Identity,Entry<?>> | put | Identity |
| Map<Identity,Entry<?>> | putAll | Map<? extends Identity,? extends Entry<?>> |
| Map<Identity,Entry<?>> | putIfAbsent | Entry<?> |
| Map<Identity,Entry<?>> | putIfAbsent | Identity |
| Map<Identity,Entry<?>> | remove | Object |
| Map<Identity,Entry<?>> | replace | Entry<?> |
| Map<Identity,Entry<?>> | replace | Identity |
| Map<Identity,Entry<?>> | replaceAll | BiFunction<? super Identity,? super Entry<?>,? extends Entry<?>> |
| Map<Identity,Object> | compute | BiFunction<? super Identity,? super Object,? extends Object> |
| Map<Identity,Object> | compute | Identity |
| Map<Identity,Object> | computeIfAbsent | Function<? super Identity,? extends Object> |
| Map<Identity,Object> | computeIfAbsent | Identity |
| Map<Identity,Object> | computeIfPresent | BiFunction<? super Identity,? super Object,? extends Object> |
| Map<Identity,Object> | computeIfPresent | Identity |
| Map<Identity,Object> | containsKey | Object |
| Map<Identity,Object> | containsValue | Object |
| Map<Identity,Object> | copyOf | Map<? extends K,? extends V> |
| Map<Identity,Object> | entry | K |
| Map<Identity,Object> | entry | V |
| Map<Identity,Object> | equals | Object |
| Map<Identity,Object> | forEach | BiConsumer<? super Identity,? super Object> |
| Map<Identity,Object> | get | Object |
| Map<Identity,Object> | getOrDefault | Object |
| Map<Identity,Object> | merge | BiFunction<? super Object,? super Object,? extends Object> |
| Map<Identity,Object> | merge | Identity |
| Map<Identity,Object> | merge | Object |
| Map<Identity,Object> | of | K |
| Map<Identity,Object> | of | V |
| Map<Identity,Object> | ofEntries | Entry<? extends K,? extends V>[] |
| Map<Identity,Object> | put | Identity |
| Map<Identity,Object> | put | Object |
| Map<Identity,Object> | putAll | Map<? extends Identity,? extends Object> |
| Map<Identity,Object> | putIfAbsent | Identity |
| Map<Identity,Object> | putIfAbsent | Object |
| Map<Identity,Object> | remove | Object |
| Map<Identity,Object> | replace | Identity |
| Map<Identity,Object> | replace | Object |
| Map<Identity,Object> | replaceAll | BiFunction<? super Identity,? super Object,? extends Object> |
| Map<K,V> | compute | BiFunction<? super K,? super V,? extends V> |
| Map<K,V> | compute | K |
| Map<K,V> | computeIfAbsent | Function<? super K,? extends V> |
Expand Down
1 change: 1 addition & 0 deletions java/ql/test-kotlin1/library-tests/reflection/reflection.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ compGenerated
| file://<external>/LongProgression.class:0:0:0:0 | spliterator | Forwarder for a Kotlin class inheriting an interface default method |
| file://<external>/LongRange.class:0:0:0:0 | forEach | Forwarder for a Kotlin class inheriting an interface default method |
| file://<external>/LongRange.class:0:0:0:0 | spliterator | Forwarder for a Kotlin class inheriting an interface default method |
| file://<external>/String.class:0:0:0:0 | getChars | Forwarder for a Kotlin class inheriting an interface default method |
| file://<external>/String.class:0:0:0:0 | isEmpty | Forwarder for a Kotlin class inheriting an interface default method |
| reflection.kt:7:49:7:54 | new Function2<Ccc,Integer,Double>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:10:38:10:42 | new KProperty1<C,Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
Expand Down
Loading