Skip to content

Commit 19e19c1

Browse files
jrivardjrivard
committed
fix issue #688 - photo upload/download mime type enforcement
1 parent 18a405e commit 19e19c1

File tree

12 files changed

+101
-108
lines changed

12 files changed

+101
-108
lines changed

‎server/src/main/java/password/pwm/AppProperty.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,8 @@ public enum AppProperty
340340
SECURITY_HTTP_PERFORM_CSRF_HEADER_CHECKS ( "security.http.performCsrfHeaderChecks" ),
341341
SECURITY_HTTP_PROMISCUOUS_ENABLE ( "security.http.promiscuousEnable" ),
342342
SECURITY_HTTP_CONFIG_CSP_HEADER ( "security.http.config.cspHeader" ),
343+
SECURITY_HTTP_USER_PHOTO_MIME_TYPES ( "security.http.permittedUserPhotoMimeTypes" ),
344+
SECURITY_HTTP_PERMITTED_URL_PATH_CHARS ( "security.http.permittedUrlPathCharacters" ),
343345
SECURITY_HTTPSSERVER_SELF_FUTURESECONDS ( "security.httpsServer.selfCert.futureSeconds" ),
344346
SECURITY_HTTPSSERVER_SELF_ALG ( "security.httpsServer.selfCert.alg" ),
345347
SECURITY_HTTPSSERVER_SELF_KEY_SIZE ( "security.httpsServer.selfCert.keySize" ),

‎server/src/main/java/password/pwm/config/AppConfig.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,12 @@ public boolean hasDbConfigured( )
315315
&& readSettingAsPassword( PwmSetting.DATABASE_PASSWORD ) != null;
316316
}
317317

318+
public List<String> permittedPhotoMimeTypes()
319+
{
320+
final String permittedMimeTypesStr = readAppProperty( AppProperty.SECURITY_HTTP_USER_PHOTO_MIME_TYPES );
321+
return List.copyOf( StringUtil.splitAndTrim( permittedMimeTypesStr, "," ) );
322+
}
323+
318324
@Override
319325
public PasswordData readSettingAsPassword( final PwmSetting setting )
320326
{

‎server/src/main/java/password/pwm/config/value/FormValue.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,6 @@ public String toDebugString( final Locale locale )
185185
}
186186
if ( formRow.getType() == FormConfiguration.Type.photo )
187187
{
188-
sb.append( " MimeTypes: " ).append( StringUtil.collectionToString( formRow.getMimeTypes() ) ).append( '\n' );
189188
sb.append( " MaxSize: " ).append( formRow.getMaximumSize() ).append( '\n' );
190189
}
191190
}

‎server/src/main/java/password/pwm/config/value/data/FormConfiguration.java

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636

3737
import java.math.BigInteger;
3838
import java.util.ArrayList;
39-
import java.util.Arrays;
4039
import java.util.Collection;
4140
import java.util.Collections;
4241
import java.util.List;
@@ -121,15 +120,6 @@ public enum Source
121120
@Builder.Default
122121
private Map<String, String> selectOptions = Collections.emptyMap();
123122

124-
@Builder.Default
125-
private List<String> mimeTypes = Arrays.asList(
126-
"image/gif",
127-
"image/png",
128-
"image/jpeg",
129-
"image/bmp",
130-
"image/webp"
131-
);
132-
133123
@Builder.Default
134124
private int maximumSize = 65000;
135125

‎server/src/main/java/password/pwm/http/PwmURL.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,12 @@ public String getPostServletPath( final PwmServletDefinition pwmServletDefinitio
426426
return "";
427427
}
428428

429+
public List<String> getPathSegments()
430+
{
431+
final String uriPath = uri.getPath();
432+
return StringUtil.splitAndTrim( uriPath, "/" );
433+
}
434+
429435
public String determinePwmServletPath( )
430436
{
431437
final String requestPath = this.pathMinusContextAndDomain();

‎server/src/main/java/password/pwm/http/ServletUtility.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,18 @@
2121
package password.pwm.http;
2222

2323
import password.pwm.PwmConstants;
24+
import password.pwm.config.AppConfig;
25+
import password.pwm.data.ImmutableByteArray;
2426
import password.pwm.error.ErrorInformation;
2527
import password.pwm.error.PwmError;
2628
import password.pwm.error.PwmUnrecoverableException;
2729
import password.pwm.util.java.JavaHelper;
30+
import password.pwm.util.java.StringUtil;
2831

2932
import javax.servlet.http.HttpServletRequest;
3033
import java.io.IOException;
34+
import java.net.URLConnection;
35+
import java.util.List;
3136

3237
public final class ServletUtility
3338
{
@@ -48,4 +53,22 @@ public static String readRequestBodyAsString( final HttpServletRequest req, fina
4853
}
4954
return value;
5055
}
56+
57+
58+
public static String mimeTypeForUserPhoto(
59+
final AppConfig configuration,
60+
final ImmutableByteArray immutableByteArray
61+
)
62+
throws IOException, PwmUnrecoverableException
63+
{
64+
final List<String> permittedMimeTypes = configuration.permittedPhotoMimeTypes();
65+
66+
final String mimeType = URLConnection.guessContentTypeFromStream( immutableByteArray.newByteArrayInputStream() );
67+
if ( !StringUtil.isEmpty( mimeType ) && permittedMimeTypes.contains( mimeType ) )
68+
{
69+
return mimeType;
70+
}
71+
final ErrorInformation errorInformation = new ErrorInformation( PwmError.ERROR_FILE_TYPE_INCORRECT, "unsupported mime type" );
72+
throw new PwmUnrecoverableException( errorInformation );
73+
}
5174
}

‎server/src/main/java/password/pwm/http/filter/RequestInitializationFilter.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
import java.util.List;
7373
import java.util.Map;
7474
import java.util.Optional;
75+
import java.util.regex.Pattern;
7576
import java.util.stream.Collectors;
7677

7778
public class RequestInitializationFilter implements Filter
@@ -488,6 +489,31 @@ public static void addStaticResponseHeaders(
488489
}
489490
}
490491

492+
private static void checkURlPathSegments( final PwmRequest pwmRequest )
493+
throws PwmUnrecoverableException
494+
{
495+
if ( pwmRequest.getURL().isResourceURL() )
496+
{
497+
return;
498+
}
499+
500+
final String checkRegexPatternString = pwmRequest.getAppConfig().readAppProperty( AppProperty.SECURITY_HTTP_PERMITTED_URL_PATH_CHARS );
501+
if ( StringUtil.isEmpty( checkRegexPatternString ) )
502+
{
503+
return;
504+
}
505+
506+
final Pattern pattern = Pattern.compile( checkRegexPatternString );
507+
for ( final String pathPart : pwmRequest.getURL().getPathSegments() )
508+
{
509+
if ( !pattern.matcher( pathPart ).matches() )
510+
{
511+
final String errorMsg = "request URL path segment contains illegal characters";
512+
final ErrorInformation errorInformation = new ErrorInformation( PwmError.ERROR_SECURITY_VIOLATION, errorMsg );
513+
throw new PwmUnrecoverableException( errorInformation );
514+
}
515+
}
516+
}
491517

492518
private static void handleRequestInitialization(
493519
final PwmRequest pwmRequest
@@ -554,6 +580,9 @@ private static void handleRequestSecurityChecks( final PwmRequest pwmRequest )
554580
// check the user's IP address
555581
checkIfSourceAddressChanged( pwmRequest );
556582

583+
// check url path segments
584+
checkURlPathSegments( pwmRequest );
585+
557586
// check total time.
558587
checkTotalSessionTime( pwmRequest );
559588

‎server/src/main/java/password/pwm/http/servlet/updateprofile/UpdateProfileServlet.java

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import password.pwm.config.PwmSetting;
3232
import password.pwm.config.profile.UpdateProfileProfile;
3333
import password.pwm.config.value.data.FormConfiguration;
34+
import password.pwm.data.ImmutableByteArray;
3435
import password.pwm.error.ErrorInformation;
3536
import password.pwm.error.PwmDataValidationException;
3637
import password.pwm.error.PwmError;
@@ -43,7 +44,7 @@
4344
import password.pwm.http.PwmRequest;
4445
import password.pwm.http.PwmRequestAttribute;
4546
import password.pwm.http.PwmSession;
46-
import password.pwm.data.ImmutableByteArray;
47+
import password.pwm.http.ServletUtility;
4748
import password.pwm.http.bean.UpdateProfileBean;
4849
import password.pwm.http.servlet.ControlledPwmServlet;
4950
import password.pwm.i18n.Message;
@@ -55,11 +56,10 @@
5556
import password.pwm.svc.token.TokenType;
5657
import password.pwm.svc.token.TokenUtil;
5758
import password.pwm.util.form.FormUtility;
58-
import password.pwm.util.java.CollectionUtil;
5959
import password.pwm.util.java.JavaHelper;
6060
import password.pwm.util.java.PwmUtil;
61-
import password.pwm.util.json.JsonFactory;
6261
import password.pwm.util.java.StringUtil;
62+
import password.pwm.util.json.JsonFactory;
6363
import password.pwm.util.logging.PwmLogger;
6464
import password.pwm.util.macro.MacroRequest;
6565
import password.pwm.ws.server.RestResultBean;
@@ -68,11 +68,9 @@
6868
import javax.servlet.annotation.WebServlet;
6969
import javax.servlet.http.HttpServletRequest;
7070
import javax.servlet.http.HttpServletResponse;
71-
import java.io.ByteArrayInputStream;
7271
import java.io.IOException;
7372
import java.io.InputStream;
7473
import java.io.OutputStream;
75-
import java.net.URLConnection;
7674
import java.util.Collection;
7775
import java.util.Collections;
7876
import java.util.List;
@@ -521,27 +519,27 @@ public ProcessStatus uploadPhoto( final PwmRequest pwmRequest ) throws ServletEx
521519
final Optional<InputStream> uploadedFile = pwmRequest.readFileUploadStream( PwmConstants.PARAM_FILE_UPLOAD );
522520
if ( uploadedFile.isPresent() )
523521
{
524-
try ( InputStream inputStream = uploadedFile.get() )
522+
523+
final ImmutableByteArray bytes = JavaHelper.copyToBytes( uploadedFile.get(), maxSize + 1 );
524+
525+
if ( bytes.size() > maxSize )
525526
{
526-
final ImmutableByteArray bytes = JavaHelper.copyToBytes( inputStream, maxSize );
527-
final String b64String = StringUtil.base64Encode( bytes.copyOf() );
528-
529-
if ( !CollectionUtil.isEmpty( formConfiguration.getMimeTypes() ) )
530-
{
531-
final String mimeType = URLConnection.guessContentTypeFromStream( bytes.newByteArrayInputStream() );
532-
if ( !formConfiguration.getMimeTypes().contains( mimeType ) )
533-
{
534-
final ErrorInformation errorInformation = new ErrorInformation( PwmError.ERROR_FILE_TYPE_INCORRECT, "incorrect file type of " + mimeType, new String[]
535-
{
536-
mimeType,
537-
}
538-
);
539-
pwmRequest.outputJsonResult( RestResultBean.fromError( errorInformation, pwmRequest ) );
540-
return ProcessStatus.Halt;
541-
}
542-
}
543-
updateProfileBean.getFormData().put( fieldName, b64String );
527+
final ErrorInformation errorInformation = new ErrorInformation( PwmError.ERROR_FILE_TOO_LARGE,
528+
"file size exceeds maximum file size (" + maxSize + ")" );
529+
pwmRequest.outputJsonResult( RestResultBean.fromError( errorInformation, pwmRequest ) );
530+
return ProcessStatus.Halt;
544531
}
532+
533+
if ( ServletUtility.mimeTypeForUserPhoto( pwmRequest.getAppConfig(), bytes ).isEmpty() )
534+
{
535+
final ErrorInformation errorInformation = new ErrorInformation( PwmError.ERROR_FILE_TYPE_INCORRECT,
536+
"unsupported mime type" );
537+
pwmRequest.outputJsonResult( RestResultBean.fromError( errorInformation, pwmRequest ) );
538+
return ProcessStatus.Halt;
539+
}
540+
541+
final String b64String = StringUtil.base64Encode( bytes.copyOf() );
542+
updateProfileBean.getFormData().put( fieldName, b64String );
545543
}
546544
}
547545

@@ -563,7 +561,8 @@ public ProcessStatus deletePhotoHandler( final PwmRequest pwmRequest ) throws Se
563561
}
564562

565563
@ActionHandler( action = "readPhoto" )
566-
public ProcessStatus readPhotoHandler( final PwmRequest pwmRequest ) throws ServletException, PwmUnrecoverableException, IOException
564+
public ProcessStatus readPhotoHandler( final PwmRequest pwmRequest )
565+
throws PwmUnrecoverableException, IOException
567566
{
568567
final String fieldName = pwmRequest.readParameterAsString( "field" );
569568
final UpdateProfileBean updateProfileBean = getBean( pwmRequest );
@@ -573,10 +572,11 @@ public ProcessStatus readPhotoHandler( final PwmRequest pwmRequest ) throws Serv
573572
{
574573
final byte[] bytes = StringUtil.base64Decode( b64value );
575574

575+
final String mimeType = ServletUtility.mimeTypeForUserPhoto( pwmRequest.getAppConfig(), ImmutableByteArray.of( bytes ) );
576+
576577
try ( OutputStream outputStream = pwmRequest.getPwmResponse().getOutputStream() )
577578
{
578579
final HttpServletResponse resp = pwmRequest.getPwmResponse().getHttpServletResponse();
579-
final String mimeType = URLConnection.guessContentTypeFromStream( new ByteArrayInputStream( bytes ) );
580580
resp.setContentType( mimeType );
581581
outputStream.write( bytes );
582582
outputStream.flush();

‎server/src/main/java/password/pwm/ldap/LdapOperationsHelper.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import password.pwm.error.PwmError;
5151
import password.pwm.error.PwmOperationalException;
5252
import password.pwm.error.PwmUnrecoverableException;
53+
import password.pwm.http.ServletUtility;
5354
import password.pwm.svc.cache.CacheKey;
5455
import password.pwm.svc.cache.CachePolicy;
5556
import password.pwm.svc.stats.EpsStatistic;
@@ -64,9 +65,7 @@
6465
import password.pwm.util.secure.PwmTrustManager;
6566

6667
import javax.net.ssl.X509TrustManager;
67-
import java.io.ByteArrayInputStream;
6868
import java.io.IOException;
69-
import java.net.URLConnection;
7069
import java.security.cert.X509Certificate;
7170
import java.time.Instant;
7271
import java.util.Arrays;
@@ -782,7 +781,7 @@ public static Optional<PhotoDataBean> readPhotoDataFromLdap(
782781
throw new PwmOperationalException( new ErrorInformation( PwmError.ERROR_SERVICE_NOT_AVAILABLE, "ldap photo attribute is not configured" ) );
783782
}
784783

785-
final byte[] photoData;
784+
final ImmutableByteArray photoData;
786785
final String mimeType;
787786
try
788787
{
@@ -792,8 +791,8 @@ public static Optional<PhotoDataBean> readPhotoDataFromLdap(
792791
{
793792
throw new PwmOperationalException( new ErrorInformation( PwmError.ERROR_SERVICE_NOT_AVAILABLE, "user has no photo data stored in LDAP attribute" ) );
794793
}
795-
photoData = photoAttributeData[ 0 ];
796-
mimeType = URLConnection.guessContentTypeFromStream( new ByteArrayInputStream( photoData ) );
794+
photoData = ImmutableByteArray.of( photoAttributeData[ 0 ] );
795+
mimeType = ServletUtility.mimeTypeForUserPhoto( domainConfig.getAppConfig(), photoData );
797796
}
798797
catch ( final IOException | ChaiOperationException e )
799798
{
@@ -803,7 +802,7 @@ public static Optional<PhotoDataBean> readPhotoDataFromLdap(
803802
{
804803
throw PwmUnrecoverableException.fromChaiException( e );
805804
}
806-
return Optional.of( new PhotoDataBean( mimeType, ImmutableByteArray.of( photoData ) ) );
805+
return Optional.of( new PhotoDataBean( mimeType, photoData ) );
807806
}
808807

809808

‎server/src/main/resources/password/pwm/AppProperty.properties

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,8 @@ security.http.forceRequestSequencing=false
315315
security.http.stripHeaderRegex=\\n|\\r|(?ism)%0A|%0D
316316
security.http.performCsrfHeaderChecks=false
317317
security.http.promiscuousEnable=false
318+
security.http.permittedUserPhotoMimeTypes=image/gif,image/png,image/jpeg
319+
security.http.permittedUrlPathCharacters=^[a-zA-Z0-9-]*$
318320
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
319321
security.httpsServer.selfCert.futureSeconds=63113904
320322
security.httpsServer.selfCert.alg=RSA

‎webapp/src/main/webapp/WEB-INF/jsp/fragment/form.jsp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@
125125
<script type="application/javascript">
126126
PWM_GLOBAL['startupFunctions'].push(function(){
127127
PWM_MAIN.addEventHandler('button-uploadPhoto-<%=loopConfiguration.getName()%>',"click",function(){
128-
var accept = '<%=StringUtil.collectionToString(loopConfiguration.getMimeTypes())%>';
128+
var accept = '<%=StringUtil.collectionToString(formPwmRequest.getAppConfig().permittedPhotoMimeTypes())%>';
129129
PWM_UPDATE.uploadPhoto('<%=loopConfiguration.getName()%>',{accept:accept});
130130
});
131131
});

0 commit comments

Comments
 (0)