improve photo upload/download mime type enforcement

jrivard · jrivard · commit 6d192dd40238 · 2023-02-02T17:42:44.000-05:00
diff --git a/server/src/main/java/password/pwm/AppProperty.java b/server/src/main/java/password/pwm/AppProperty.java
@@ -325,6 +325,8 @@ public enum AppProperty
     SECURITY_HTTP_PERFORM_CSRF_HEADER_CHECKS        ( "security.http.performCsrfHeaderChecks" ),
     SECURITY_HTTP_PROMISCUOUS_ENABLE                ( "security.http.promiscuousEnable" ),
     SECURITY_HTTP_CONFIG_CSP_HEADER                 ( "security.http.config.cspHeader" ),
+    SECURITY_HTTP_USER_PHOTO_MIME_TYPES             ( "security.http.permittedUserPhotoMimeTypes" ),
+    SECURITY_HTTP_PERMITTED_URL_PATH_CHARS          ( "security.http.permittedUrlPathCharacters" ),
     SECURITY_HTTPSSERVER_SELF_FUTURESECONDS         ( "security.httpsServer.selfCert.futureSeconds" ),
     SECURITY_HTTPSSERVER_SELF_ALG                   ( "security.httpsServer.selfCert.alg" ),
     SECURITY_HTTPSSERVER_SELF_KEY_SIZE              ( "security.httpsServer.selfCert.keySize" ),
diff --git a/server/src/main/java/password/pwm/config/Configuration.java b/server/src/main/java/password/pwm/config/Configuration.java
@@ -403,6 +403,11 @@ public LdapProfile getDefaultLdapProfile( ) throws PwmUnrecoverableException
         return getLdapProfiles().values().iterator().next();
     }
 
+    public List<String> permittedPhotoMimeTypes()
+    {
+        final String permittedMimeTypesStr = readAppProperty( AppProperty.SECURITY_HTTP_USER_PHOTO_MIME_TYPES );
+        return List.copyOf( StringUtil.splitAndTrim( permittedMimeTypesStr, "," ) );
+    }
 
     public List<Locale> getKnownLocales( )
     {
diff --git a/server/src/main/java/password/pwm/config/value/FormValue.java b/server/src/main/java/password/pwm/config/value/FormValue.java
@@ -191,7 +191,6 @@ public String toDebugString( final Locale locale )
                 }
                 if ( formRow.getType() == FormConfiguration.Type.photo )
                 {
-                    sb.append( " MimeTypes: " ).append( StringUtil.collectionToString( formRow.getMimeTypes() ) ).append( "\n" );
                     sb.append( " MaxSize: " ).append( formRow.getMaximumSize() ).append( "\n" );
                 }
 
diff --git a/server/src/main/java/password/pwm/config/value/data/FormConfiguration.java b/server/src/main/java/password/pwm/config/value/data/FormConfiguration.java
@@ -38,7 +38,6 @@
 import java.io.Serializable;
 import java.math.BigInteger;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
@@ -123,15 +122,6 @@ public enum Source
     @Builder.Default
     private Map<String, String> selectOptions = Collections.emptyMap();
 
-    @Builder.Default
-    private List<String> mimeTypes = Arrays.asList(
-            "image/gif",
-            "image/png",
-            "image/jpeg",
-            "image/bmp",
-            "image/webp"
-    );
-
     @Builder.Default
     private int maximumSize = 65000;
 
diff --git a/server/src/main/java/password/pwm/http/PwmURL.java b/server/src/main/java/password/pwm/http/PwmURL.java
@@ -423,6 +423,12 @@ public String getPostServletPath( final PwmServletDefinition pwmServletDefinitio
         return "";
     }
 
+    public List<String> getPathSegments()
+    {
+        final String uriPath = uri.getPath();
+        return StringUtil.splitAndTrim( uriPath, "/" );
+    }
+
     public String determinePwmServletPath( )
     {
         final String requestPath = this.uri.getPath();
diff --git a/server/src/main/java/password/pwm/http/filter/RequestInitializationFilter.java b/server/src/main/java/password/pwm/http/filter/RequestInitializationFilter.java
@@ -73,6 +73,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 public class RequestInitializationFilter implements Filter
 {
@@ -585,6 +586,9 @@ private static void handleRequestSecurityChecks( final PwmRequest pwmRequest )
         // check the user's IP address
         checkIfSourceAddressChanged( pwmRequest );
 
+        // check url path segments
+        checkURlPathSegments( pwmRequest );
+
         // check total time.
         checkTotalSessionTime( pwmRequest );
 
@@ -718,6 +722,31 @@ private static void checkSourceNetworkAddress( final PwmRequest pwmRequest )
         }
     }
 
+    private static void checkURlPathSegments( final PwmRequest pwmRequest )
+            throws PwmUnrecoverableException
+    {
+        if ( pwmRequest.getURL().isResourceURL() )
+        {
+            return;
+        }
+
+        final String checkRegexPatternString = pwmRequest.getConfig().readAppProperty( AppProperty.SECURITY_HTTP_PERMITTED_URL_PATH_CHARS );
+        if ( StringUtil.isEmpty( checkRegexPatternString ) )
+        {
+            return;
+        }
+
+        final Pattern pattern = Pattern.compile( checkRegexPatternString );
+        for ( final String pathPart : pwmRequest.getURL().getPathSegments() )
+        {
+            if ( !pattern.matcher( pathPart ).matches() )
+            {
+                final String errorMsg = "request URL path segment contains illegal characters";
+                final ErrorInformation errorInformation = new ErrorInformation( PwmError.ERROR_SECURITY_VIOLATION, errorMsg );
+                throw new PwmUnrecoverableException( errorInformation );
+            }
+        }
+    }
 
     private static void checkCsrfHeader( final PwmRequest pwmRequest )
             throws PwmUnrecoverableException
diff --git a/server/src/main/java/password/pwm/http/servlet/updateprofile/UpdateProfileServlet.java b/server/src/main/java/password/pwm/http/servlet/updateprofile/UpdateProfileServlet.java
@@ -43,6 +43,7 @@
 import password.pwm.http.PwmRequest;
 import password.pwm.http.PwmRequestAttribute;
 import password.pwm.http.PwmSession;
+import password.pwm.http.bean.ImmutableByteArray;
 import password.pwm.http.bean.UpdateProfileBean;
 import password.pwm.http.servlet.ControlledPwmServlet;
 import password.pwm.i18n.Message;
@@ -52,6 +53,7 @@
 import password.pwm.svc.token.TokenService;
 import password.pwm.svc.token.TokenType;
 import password.pwm.svc.token.TokenUtil;
+import password.pwm.util.ServletUtility;
 import password.pwm.util.form.FormUtility;
 import password.pwm.util.java.JavaHelper;
 import password.pwm.util.java.JsonUtil;
@@ -64,13 +66,11 @@
 import javax.servlet.annotation.WebServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.Serializable;
-import java.net.URLConnection;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
@@ -522,22 +522,14 @@ public ProcessStatus uploadPhoto( final PwmRequest pwmRequest ) throws ServletEx
                     return ProcessStatus.Halt;
                 }
 
-                final String b64String = StringUtil.base64Encode( bytes );
-
-                if ( !JavaHelper.isEmpty( formConfiguration.getMimeTypes() ) )
+                if ( ServletUtility.mimeTypeForUserPhoto( pwmRequest.getConfig(), ImmutableByteArray.of( bytes ) ).isEmpty() )
                 {
-                    final String mimeType = URLConnection.guessContentTypeFromStream( new ByteArrayInputStream( bytes ) );
-                    if ( !formConfiguration.getMimeTypes().contains( mimeType ) )
-                    {
-                        final ErrorInformation errorInformation = new ErrorInformation( PwmError.ERROR_FILE_TYPE_INCORRECT, "incorrect file type of " + mimeType, new String[]
-                                {
-                                        mimeType,
-                                }
-                        );
-                        pwmRequest.outputJsonResult( RestResultBean.fromError( errorInformation, pwmRequest ) );
-                        return ProcessStatus.Halt;
-                    }
+                    final ErrorInformation errorInformation = new ErrorInformation( PwmError.ERROR_FILE_TYPE_INCORRECT, "unsupported mime type" );
+                    pwmRequest.outputJsonResult( RestResultBean.fromError( errorInformation, pwmRequest ) );
+                    return ProcessStatus.Halt;
                 }
+
+                final String b64String = StringUtil.base64Encode( bytes );
                 updateProfileBean.getFormData().put( fieldName, b64String );
             }
         }
@@ -560,7 +552,8 @@ public ProcessStatus deletePhotoHandler( final PwmRequest pwmRequest ) throws Se
     }
 
     @ActionHandler( action = "readPhoto" )
-    public ProcessStatus readPhotoHandler( final PwmRequest pwmRequest ) throws ServletException, PwmUnrecoverableException, IOException
+    public ProcessStatus readPhotoHandler( final PwmRequest pwmRequest )
+            throws ServletException, PwmUnrecoverableException, IOException
     {
         final String fieldName = pwmRequest.readParameterAsString( "field" );
         final UpdateProfileBean updateProfileBean = getBean( pwmRequest );
@@ -570,10 +563,11 @@ public ProcessStatus readPhotoHandler( final PwmRequest pwmRequest ) throws Serv
         {
             final byte[] bytes = StringUtil.base64Decode( b64value );
 
+            final String mimeType = ServletUtility.mimeTypeForUserPhoto( pwmRequest.getConfig(), ImmutableByteArray.of( bytes ) );
+
             try ( OutputStream outputStream = pwmRequest.getPwmResponse().getOutputStream() )
             {
                 final HttpServletResponse resp = pwmRequest.getPwmResponse().getHttpServletResponse();
-                final String mimeType = URLConnection.guessContentTypeFromStream( new ByteArrayInputStream( bytes ) );
                 resp.setContentType( mimeType );
                 outputStream.write( bytes );
                 outputStream.flush();
diff --git a/server/src/main/java/password/pwm/ldap/LdapOperationsHelper.java b/server/src/main/java/password/pwm/ldap/LdapOperationsHelper.java
@@ -57,6 +57,7 @@
 import password.pwm.svc.stats.Statistic;
 import password.pwm.svc.stats.StatisticsManager;
 import password.pwm.util.PasswordData;
+import password.pwm.util.ServletUtility;
 import password.pwm.util.i18n.LocaleHelper;
 import password.pwm.util.java.StringUtil;
 import password.pwm.util.java.TimeDuration;
@@ -65,9 +66,7 @@
 import password.pwm.util.secure.PwmTrustManager;
 
 import javax.net.ssl.X509TrustManager;
-import java.io.ByteArrayInputStream;
 import java.io.IOException;
-import java.net.URLConnection;
 import java.security.cert.X509Certificate;
 import java.time.Instant;
 import java.util.Arrays;
@@ -940,7 +939,7 @@ public static Optional<PhotoDataBean> readPhotoDataFromLdap(
                 throw new PwmOperationalException( new ErrorInformation( PwmError.ERROR_SERVICE_NOT_AVAILABLE, "user has no photo data stored in LDAP attribute" ) );
             }
             photoData = photoAttributeData[ 0 ];
-            mimeType = URLConnection.guessContentTypeFromStream( new ByteArrayInputStream( photoData ) );
+            mimeType = ServletUtility.mimeTypeForUserPhoto( configuration, ImmutableByteArray.of( photoData ) );
         }
         catch ( final IOException | ChaiOperationException e )
         {
diff --git a/server/src/main/java/password/pwm/util/ServletUtility.java b/server/src/main/java/password/pwm/util/ServletUtility.java
@@ -22,15 +22,20 @@
 
 import org.apache.commons.io.IOUtils;
 import password.pwm.PwmConstants;
+import password.pwm.config.Configuration;
 import password.pwm.error.ErrorInformation;
 import password.pwm.error.PwmError;
 import password.pwm.error.PwmUnrecoverableException;
+import password.pwm.http.bean.ImmutableByteArray;
+import password.pwm.util.java.StringUtil;
 
 import javax.servlet.http.HttpServletRequest;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.io.StringWriter;
+import java.net.URLConnection;
+import java.util.List;
 
 public final class ServletUtility
 {
@@ -69,4 +74,21 @@ public static String readRequestBodyAsString( final HttpServletRequest req, fina
         }
         return stringValue;
     }
+
+    public static String mimeTypeForUserPhoto(
+            final Configuration configuration,
+            final ImmutableByteArray immutableByteArray
+    )
+            throws IOException, PwmUnrecoverableException
+    {
+        final List<String> permittedMimeTypes = configuration.permittedPhotoMimeTypes();
+
+        final String mimeType = URLConnection.guessContentTypeFromStream( immutableByteArray.newByteArrayInputStream() );
+        if ( !StringUtil.isEmpty( mimeType ) && permittedMimeTypes.contains( mimeType ) )
+        {
+            return mimeType;
+        }
+        final ErrorInformation errorInformation = new ErrorInformation( PwmError.ERROR_FILE_TYPE_INCORRECT, "unsupported mime type" );
+        throw new PwmUnrecoverableException( errorInformation );
+    }
 }
diff --git a/server/src/main/resources/password/pwm/AppProperty.properties b/server/src/main/resources/password/pwm/AppProperty.properties
@@ -304,6 +304,8 @@ security.http.forceRequestSequencing=false
 security.http.stripHeaderRegex=\\n|\\r|(?ism)%0A|%0D
 security.http.performCsrfHeaderChecks=false
 security.http.promiscuousEnable=false
+security.http.permittedUserPhotoMimeTypes=image/gif,image/png,image/jpeg
+security.http.permittedUrlPathCharacters=^[a-zA-Z0-9-]*$
 security.http.config.cspHeader=default-src 'self'; object-src 'none'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-eval' 'unsafe-inline'; report-uri @PwmContextPath@/public/command/cspReport
 security.httpsServer.selfCert.futureSeconds=63113904
 security.httpsServer.selfCert.alg=RSA
diff --git a/webapp/src/main/webapp/WEB-INF/jsp/fragment/form.jsp b/webapp/src/main/webapp/WEB-INF/jsp/fragment/form.jsp
@@ -124,7 +124,7 @@
                 <script type="application/javascript">
                     PWM_GLOBAL['startupFunctions'].push(function(){
                         PWM_MAIN.addEventHandler('button-uploadPhoto-<%=loopConfiguration.getName()%>',"click",function(){
-                            var accept = '<%=StringUtil.collectionToString(loopConfiguration.getMimeTypes())%>';
+                            var accept = '<%=StringUtil.collectionToString(formPwmRequest.getConfig().permittedPhotoMimeTypes())%>';
                             PWM_UPDATE.uploadPhoto('<%=loopConfiguration.getName()%>',{accept:accept});
                         });
                     });
diff --git a/webapp/src/main/webapp/public/resources/js/configeditor-settings-form.js b/webapp/src/main/webapp/public/resources/js/configeditor-settings-form.js
@@ -33,7 +33,6 @@ FormTableHandler.newRowValue = {
     javascript:'',
     regex:'',
     source:'ldap',
-    mimeTypes:['image/gif','image/png','image/jpeg','image/bmp','image/webp'],
     maximumSize:65000
 };
 
@@ -288,7 +287,7 @@ FormTableHandler.showOptionsDialog = function(keyName, iteration) {
         bodyText += '</tr><tr>';
     }
 
-    if ('select' in options) {
+    if (currentValue['type'] === 'select') {
         bodyText += '<td class="key">Select Options</td><td><button class="btn" id="' + inputID + 'editOptionsButton"><span class="btn-icon pwm-icon pwm-icon-list-ul"/> Edit</button></td>';
         bodyText += '</tr>';
     }
@@ -302,9 +301,6 @@ FormTableHandler.showOptionsDialog = function(keyName, iteration) {
     }
 
     if (currentValue['type'] === 'photo') {
-        bodyText += '<td class="key">MimeTypes</td><td><button class="btn" id="' + inputID + 'editMimeTypesButton"><span class="btn-icon pwm-icon pwm-icon-list-ul"/> Edit</button></td>';
-        bodyText += '</tr>';
-
         bodyText += '<td class="key">Maximum Size (bytes)</td><td><input min="0" pattern="[0-9]{1,10}" max="10000000" style="width: 90px" type="number" id="' + inputID + 'maximumSize' + '"/></td>';
         bodyText += '</tr><tr>';
     }
@@ -587,18 +583,6 @@ FormTableHandler.showMimeTypesDialog = function(keyName, iteration) {
     var bodyText = '';
     bodyText += '<table class="noborder" id="' + inputID + 'table"">';
     bodyText += '<tr>';
-    bodyText += '</tr><tr>';
-    for (var optionName in PWM_VAR['clientSettingCache'][keyName][iteration]['mimeTypes']) {
-        (function(optionName) {
-            var value = PWM_VAR['clientSettingCache'][keyName][iteration]['mimeTypes'][optionName];
-            var optionID = inputID + optionName;
-            bodyText += '<td><div class="noWrapTextBox">' + value + '</div></td>';
-            bodyText += '<td class="noborder" style="">';
-            bodyText += '<span id="' + optionID + '-removeButton" class="delete-row-icon action-icon pwm-icon pwm-icon-times"></span>';
-            bodyText += '</td>';
-            bodyText += '</tr><tr>';
-        }(optionName));
-    }
     bodyText += '</tr></table>';
     bodyText += '<br/><br/><br/>';
     bodyText += '<input class="configStringInput" pattern=".*/.*" style="width:200px" type="text" placeholder="Value" required id="addValue"/>';
@@ -611,28 +595,4 @@ FormTableHandler.showMimeTypesDialog = function(keyName, iteration) {
             FormTableHandler.showOptionsDialog(keyName,iteration);
         }
     });
-
-    for (var optionName in PWM_VAR['clientSettingCache'][keyName][iteration]['mimeTypes']) {
-        (function(optionName) {
-            var optionID = inputID + optionName;
-            PWM_MAIN.addEventHandler(optionID + '-removeButton','click',function(){
-                delete PWM_VAR['clientSettingCache'][keyName][iteration]['mimeTypes'][optionName];
-                FormTableHandler.write(keyName);
-                FormTableHandler.showMimeTypesDialog(keyName, iteration);
-            });
-        }(optionName));
-    }
-
-    PWM_MAIN.addEventHandler('addItemButton','click',function(){
-        var value = PWM_MAIN.getObject('addValue').value;
-
-        if (value === null || value.length < 1) {
-            alert('Value field is required');
-            return;
-        }
-
-        PWM_VAR['clientSettingCache'][keyName][iteration]['mimeTypes'].push(value);
-        FormTableHandler.write(keyName);
-        FormTableHandler.showMimeTypesDialog(keyName, iteration);
-    });
 };

Original file line number	Diff line number	Diff line change
`@@ -403,6 +403,11 @@ public LdapProfile getDefaultLdapProfile( ) throws PwmUnrecoverableException`
`403`	`403`	`return getLdapProfiles().values().iterator().next();`
`404`	`404`	`}`
`405`	`405`
	`406`	`+ public List<String> permittedPhotoMimeTypes()`
	`407`	`+ {`
	`408`	`+ final String permittedMimeTypesStr = readAppProperty( AppProperty.SECURITY_HTTP_USER_PHOTO_MIME_TYPES );`
	`409`	`+ return List.copyOf( StringUtil.splitAndTrim( permittedMimeTypesStr, "," ) );`
	`410`	`+ }`
`406`	`411`
`407`	`412`	`public List<Locale> getKnownLocales( )`
`408`	`413`	`{`
Original file line number	Diff line number	Diff line change
`@@ -191,7 +191,6 @@ public String toDebugString( final Locale locale )`
`191`	`191`	`}`
`192`	`192`	`if ( formRow.getType() == FormConfiguration.Type.photo )`
`193`	`193`	`{`
`194`		`- sb.append( " MimeTypes: " ).append( StringUtil.collectionToString( formRow.getMimeTypes() ) ).append( "\n" );`
`195`	`194`	`sb.append( " MaxSize: " ).append( formRow.getMaximumSize() ).append( "\n" );`
`196`	`195`	`}`
`197`	`196`
Original file line number	Diff line number	Diff line change
`@@ -423,6 +423,12 @@ public String getPostServletPath( final PwmServletDefinition pwmServletDefinitio`
`423`	`423`	`return "";`
`424`	`424`	`}`
`425`	`425`
	`426`	`+ public List<String> getPathSegments()`
	`427`	`+ {`
	`428`	`+ final String uriPath = uri.getPath();`
	`429`	`+ return StringUtil.splitAndTrim( uriPath, "/" );`
	`430`	`+ }`
	`431`	`+`
`426`	`432`	`public String determinePwmServletPath( )`
`427`	`433`	`{`
`428`	`434`	`final String requestPath = this.uri.getPath();`