when I run mvn clean install -DskipTests I can build the project and it do see

[INFO] AppEngine :: runtime-deployment 2.0.11-SNAPSHOT .... SUCCESS [  1.771 s]

However I should be seeing jars inside of the runtime/deployment/target/runtime-deployment-2.0.11-SNAPSHOT directory.

If I then run mcist -pl :runtime-deployment I now see runtime-impl.jar, runtime-main.jar and runtime-shared.jar inside of this directory.

I wasn't able to cleanly import the project into intellij since the dependency from the shared_sdk was not for the API built in the project. This change gives me a clean import and build.

When building the project with parallel build:

mvn clean install -DskipTests -T 1C

I see BUILD FAILURE:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-dependency-plugin:3.2.0:unpack (unpack) on project appengine-java-sdk: Artifact has not been packaged yet. When used on reactor artifact, unpack should be executed after packaging: see MDEP-98. -> [Help 1]

PS C:\Users\gjones\IdeaProjects\AppEngine> C:\Users\gjones\AppData\Local\google\ct4j-cloud-sdk\LATEST\google-cloud-sdk\bin\java_dev_appserver.cmd .\
Executing [C:\Users\gjones\scoop\apps\temurin17-jdk\current\bin\java, -Dfile.encoding=UTF-8, -classpath, C:\Users\gjones\AppData\Local\Google\ct4j-cloud-sdk\LATEST\google-cloud-sdk\platform\google_appengine\google\appengine\tools\java\lib\appengine-tools-api.jar, com.google.appengine.tools.development.DevAppServerMain, --property=kickstart.user.dir=C:\Users\gjones\IdeaProjects\AppEngine, --no_java_agent, --promote_yaml, C:\Users\gjones\IdeaProjects\AppEngine\.]
java.lang.RuntimeException: Unable to create a DevAppServer
        at com.google.appengine.tools.development.DevAppServerFactory.doCreateDevAppServer(DevAppServerFactory.java:378)
        at com.google.appengine.tools.development.DevAppServerFactory.createDevAppServer(DevAppServerFactory.java:310)
        at com.google.appengine.tools.development.DevAppServerMain$StartAction.apply(DevAppServerMain.java:384)
        at com.google.appengine.tools.util.Parser$ParseResult.applyArgs(Parser.java:58)
        at com.google.appengine.tools.development.DevAppServerMain.run(DevAppServerMain.java:258)
        at com.google.appengine.tools.development.DevAppServerMain.main(DevAppServerMain.java:249)
Caused by: java.lang.ExceptionInInitializerError
        at com.google.appengine.tools.development.DevAppServerImpl.<init>(DevAppServerImpl.java:135)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
        at com.google.appengine.tools.development.DevAppServerFactory.doCreateDevAppServer(DevAppServerFactory.java:363)
        ... 5 more
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make static java.net.URLStreamHandler java.net.URL.getURLStreamHandler(java.lang.String) accessible: module java.base does not "opens java.net" to unnamed module @6c64cb25
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
        at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
        at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:199)
        at java.base/java.lang.reflect.Method.setAccessible(Method.java:193)
        at com.google.appengine.tools.development.StreamHandlerFactory.<clinit>(StreamHandlerFactory.java:52)
        ... 12 more

Security Vulnerability Fix

This pull request fixes a Zip Slip vulnerability either due to an insufficient, or missing guard when unzipping zip files.

Even if you deem, as the maintainer of this project, this is not necessarily fixing a security vulnerability, it is still, most likely, a valid security hardening.

Preamble

Impact

This issue allows a malicious zip file to potentially break out of the expected destination directory, writing contents into arbitrary locations on the file system. Overwriting certain files/directories could allow an attacker to achieve remote code execution on a target system by exploiting this vulnerability.

Why?

The best description of Zip-Slip can be found in the white paper published by Snyk: Zip Slip Vulnerability

But I had a guard in place, why wasn't it sufficient?

If the changes you see are a change to the guard, not the addition of a new guard, this is probably because this code contains a Zip-Slip vulnerability due to a partial path traversal vulnerability.

To demonstrate this vulnerability, consider "/usr/outnot".startsWith("/usr/out"). The check is bypassed although /outnot is not under the /out directory. It's important to understand that the terminating slash may be removed when using various String representations of the File object. For example, on Linux, println(new File("/var")) will print /var, but println(new File("/var", "/") will print /var/; however, println(new File("/var", "/").getCanonicalPath()) will print /var.

The Fix

Implementing a guard comparing paths with the method java.nio.files.Path#startsWith will adequately protect against this vulnerability.

For example: file.getCanonicalFile().toPath().startsWith(BASE_DIRECTORY) or file.getCanonicalFile().toPath().startsWith(BASE_DIRECTORY_FILE.getCanonicalFile().toPath())

Other Examples

• CVE-2018-1002201 - zeroturnaround/zt-zip
• CVE-2018-1002202 - srikanth-lingala/zip4j
• CVE-2018-8009 - apache/hadoop

:arrow_right: Vulnerability Disclosure :arrow_left:

:wave: Vulnerability disclosure is a super important part of the vulnerability handling process and should not be skipped! This may be completely new to you, and that's okay, I'm here to assist!

First question, do we need to perform vulnerability disclosure? It depends!

1. Is the vulnerable code only in tests or example code? No disclosure required!
2. Is the vulnerable code in code shipped to your end users? Vulnerability disclosure is probably required!

For partial path traversal, consider if user-supplied input could ever flow to this logic. If user-supplied input could reach this conditional, it's insufficient and, as such, most likely a vulnerability.

Vulnerability Disclosure How-To

You have a few options options to perform vulnerability disclosure. However, I'd like to suggest the following 2 options:

1. Request a CVE number from GitHub by creating a repository-level GitHub Security Advisory. This has the advantage that, if you provide sufficient information, GitHub will automatically generate Dependabot alerts for your downstream consumers, resolving this vulnerability more quickly.
2. Reach out to the team at Snyk to assist with CVE issuance. They can be reached at the Snyk's Disclosure Email. Note: Please include JLLeitschuh Disclosure in the subject of your email so it is not missed.

Detecting this and Future Vulnerabilities

You can automatically detect future vulnerabilities like this by enabling the free (for open-source) GitHub Action.

I'm not an employee of GitHub, I'm simply an open-source security researcher.

Source

This contribution was automatically generated with an OpenRewrite refactoring recipe, which was lovingly handcrafted to bring this security fix to your repository.

The source code that generated this PR can be found here: Zip Slip

Why didn't you disclose privately (ie. coordinated disclosure)?

This PR was automatically generated, in-bulk, and sent to this project as well as many others, all at the same time.

This is technically what is called a "Full Disclosure" in vulnerability disclosure, and I agree it's less than ideal. If GitHub offered a way to create private pull requests to submit pull requests, I'd leverage it, but that infrastructure, sadly, doesn't exist yet.

The problem is that, as an open source software security researcher, I (exactly like open source maintainers), I only have so much time in a day. I'm able to find vulnerabilities impacting hundreds, or sometimes thousands of open source projects with tools like GitHub Code Search and CodeQL. The problem is that my knowledge of vulnerabilities doesn't scale very well.

Individualized vulnerability disclosure takes time and care. It's a long and tedious process, and I have a significant amount of experience with it (I have over 50 CVEs to my name). Even tracking down the reporting channel (email, Jira, etc..) can take time and isn't automatable. Unfortunately, when facing problems of this scale, individual reporting doesn't work well either.

Additionally, if I just spam out emails or issues, I'll just overwhelm already over-taxed maintainers, I don't want to do this either.

By creating a pull request, I am aiming to provide maintainers something highly actionable to actually fix the identified vulnerability; a pull request.

There's a larger discussion on this topic that can be found here: https://github.com/JLLeitschuh/security-research/discussions/12

Opting Out

If you'd like to opt out of future automated security vulnerability fixes like this, please consider adding a file called .github/GH-ROBOTS.txt to your repository with the line:

User-agent: JLLeitschuh/security-research
Disallow: *

This bot will respect the ROBOTS.txt format for future contributions.

Alternatively, if this project is no longer actively maintained, consider archiving the repository.

CLA Requirements

This section is only relevant if your project requires contributors to sign a Contributor License Agreement (CLA) for external contributions.

It is unlikely that I'll be able to directly sign CLAs. However, all contributed commits are already automatically signed off.

The meaning of a signoff depends on the project, but it typically certifies that committer has the rights to submit this work under the same license and agrees to a Developer Certificate of Origin (see https://developercertificate.org/ for more information).

- Git Commit Signoff documentation

If signing your organization's CLA is a strict-requirement for merging this contribution, please feel free to close this PR.

Sponsorship & Support

This contribution is sponsored by HUMAN Security Inc. and the new Dan Kaminsky Fellowship, a fellowship created to celebrate Dan's memory and legacy by funding open-source work that makes the world a better (and more secure) place.

This PR was generated by Moderne, a free-for-open source SaaS offering that uses format-preserving AST transformations to fix bugs, standardize code style, apply best practices, migrate library versions, and fix common security vulnerabilities at scale.

Tracking

All PR's generated as part of this fix are tracked here: https://github.com/JLLeitschuh/security-research/issues/16

The test was dependent on JVM specific formatting of Dates. The test passed on JDK8, but fails on JDK11. Changed to reparse the Date string using the same format used to generate it and then explicitly test for the time interval in ms.

Ignore files generated during the build of the project, this is the list of files that I see after a full build:

• appengine_testing_tests/WEB-INF/appengine-generated/datastore-indexes-auto.xml
• appengine_testing_tests/WEB-INF/appengine-generated/local_db.bin
• applications/proberapp/${appengine.runtime.location}/runtime-impl.jar
• applications/proberapp/${appengine.runtime.location}/runtime-main.jar
• applications/proberapp/${appengine.runtime.location}/runtime-shared.jar

Fix missing flag fragment from previous change.

Default java8 build system did not detect this, only github worflows using java11 and java17, which is the good intended way of testing with other JDKs.

There appears to be some confusion about the Servlets defined in runtime/impl/src/main/java/com/google/apphosting/runtime/jetty94/AppEngineWebAppContext.java

We have a section of code that attempts to remove deprecated Servlets and then ensure that some Servlets are added ...

https://github.com/GoogleCloudPlatform/appengine-java-standard/blob/main/runtime/impl/src/main/java/com/google/apphosting/runtime/jetty94/AppEngineWebAppContext.java#L225-L256

https://github.com/GoogleCloudPlatform/appengine-java-standard/blob/bcb69c04b532da33c11da826a00657ced2b0b670/runtime/impl/src/main/java/com/google/apphosting/runtime/jetty94/AppEngineWebAppContext.java#L225-L256

But the listed DEPRECATED_SERVLETS_FILTERS appear to be the same ones being added (ensure) back to the WebAppContext.

https://github.com/GoogleCloudPlatform/appengine-java-standard/blob/main/runtime/impl/src/main/java/com/google/apphosting/runtime/jetty94/AppEngineWebAppContext.java#L97-L118

https://github.com/GoogleCloudPlatform/appengine-java-standard/blob/bcb69c04b532da33c11da826a00657ced2b0b670/runtime/impl/src/main/java/com/google/apphosting/runtime/jetty94/AppEngineWebAppContext.java#L97-L118

Is that what you want to do?

If that entire section was removed and simply defined within a custom webdefault.xml, and specified in the AppEngineWebAppContext.setDefaultDescriptor(String) then all of those servlets will be added and flagged with mapping.isDefault = true allowing them to be overridden by the deployed webapps.

Hello,

I'm using the datastore callbacks to log the entity groups involved in a single transaction. The callbacks fire correctly but I've noticed in the logs some elements for which I explicitly passed null as a transaction.

So, digging the code I've found that the context passed to datastore callbacks is not populated with the transaction received in the get/put/delete/prepare methods, but is taken from an outer context.

  @Override
  public Future<Map<Key, Entity>> get(@Nullable Transaction txn, Iterable<Key> keys) {
    if (keys == null) {
      throw new NullPointerException("keys cannot be null");
    }

    // TODO: The handling of the Keys is pretty ugly.  We get an Iterable from the
    // user.  We need to copy this to a random access list (that we can mutate) for the PreGet
    // stuff. We will also convert it to a HashSet to support contains checks (for the RemoteApi
    // workaround).  There are also some O(N) calls to remove Keys from a List.
    List<Key> keyList = Lists.newArrayList(keys);

    // Allocate the Map that will receive the result of the RPC here so that PreGet callbacks can
    // add results.
    Map<Key, Entity> resultMap = new HashMap<Key, Entity>();
    PreGetContext preGetContext = new PreGetContext(this, keyList, resultMap);
    datastoreServiceConfig.getDatastoreCallbacks().executePreGetCallbacks(preGetContext);

    // Don't fetch anything from datastore that was provided by the preGet hooks.
    keyList.removeAll(resultMap.keySet());

    // Send the RPC(s).
    Future<Map<Key, Entity>> result = doBatchGet(txn, Sets.newLinkedHashSet(keyList), resultMap);

    // Invoke the user post-get callbacks.
    return new PostLoadFuture(result, datastoreServiceConfig.getDatastoreCallbacks(), this);
  }

For example, in the get method the context is created using this as transaction provider whilst the transaction is explicitly passed in the method, this lead to a wrong current transaction inside the callback.

Thank you Stefano

My automation found that there are duplicate classes between com.google.appengine:appengine-api-1.0-sdk and com.google.appengine:appengine-api-stubs and other artifacts.

Failing test due to duplicate classes found on classpath:
Duplicate class com.google.apphosting.base.protos.api.ApiBasePb found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.
Duplicate class com.google.appengine.repackaged.com.google.storage.onestore.v3.proto2api.OnestoreEntity found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.
Duplicate class com.google.appengine.repackaged.com.google.storage.onestore.v3.proto2api.OnestoreSnapshot found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.
Duplicate class com.google.appengine.repackaged.com.google.storage.onestore.v3.proto2api.OnestoreAction found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.
Duplicate class com.google.appengine.repackaged.org.apache.commons.logging.impl.Log4JLogger found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.
Duplicate class com.google.appengine.repackaged.org.apache.commons.logging.impl.LogFactoryImpl found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.
Duplicate class com.google.appengine.repackaged.org.apache.commons.logging.impl.LogKitLogger found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.
Duplicate class com.google.appengine.repackaged.org.apache.commons.logging.impl.NoOpLog found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.
Duplicate class com.google.appengine.repackaged.org.apache.commons.logging.impl.SimpleLog found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.
Duplicate class com.google.appengine.repackaged.org.apache.commons.logging.impl.AvalonLogger found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.
Duplicate class com.google.appengine.repackaged.org.apache.commons.logging.impl.Jdk14Logger found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.
Duplicate class com.google.appengine.repackaged.org.apache.commons.logging.Log found in classpath. Found in artifacts com.google.appengine:appengine-api-1.0-sdk:2.0.4 and com.google.appengine:appengine-api-stubs:2.0.4.

Full output https://github.com/GoogleCloudPlatform/cloud-opensource-java/runs/6132396380?check_suite_focus=true

As long as the duplicate classes identical in users' environment, I don't expect they cause real issues. However, it's not ideal to have duplicate classes in different artifacts, because they would lead to complex dependency conflicts (NoSuchMethodErrors, NoClassDefFoundErrors) when users' environment has different versions of the duplicate classes.

So far the duplicate classes are not causing any problem to me. Sharing my observation here.